Repository navigation
[stable35] feat(database): add a database page with the DB Doctor checks - #1130
Merged
Merged
Conversation
backportbot
Bot
added
3. to review
AI assisted
karlitschek
approved these changes
Sep 9, 2026
The Monitoring app reports what the database is, never whether it is configured well, and the answer to that already exists in Nextcloud's DB Doctor app. This ports its check engine into a third page, between Status and Background jobs. What came across: - the rule engine (Rule, RuleResult, RuleSet, the expression evaluator and the advisor) and the probe that snapshots the server - the MySQL/MariaDB rule set, ported in DB Doctor from phpMyAdmin's advisor and carrying its GPL-2.0-or-later attribution, plus the PostgreSQL and Nextcloud schema rules - the dialog for pointing the checks at a separate connection, so they can run as a user with rights Nextcloud itself does not need. It stores under serverinfo's own app config, with the password in the credentials manager - the live connections, throughput and latency boxes What deliberately did not: - the score, grade, mascot, history and trend charts. This is a page about what is wrong, not a gamified dashboard - everything that writes. Failing checks offer the config snippet to paste into my.cnf or postgresql.conf, but nothing applies a value: a bundled monitoring app has no business holding write access to the database The checks run once when the page is first opened rather than on a button, and the page keeps its results when you switch tabs and come back. Two departures from how DB Doctor does it. Its dashboard polls /insights/metrics every three seconds and /check/ping once a second; here both are one endpoint that returns the metrics and times its own round trip, because the request cycle costs orders of magnitude more than the queries inside it. And both live endpoints close the PHP session before doing their work: holding it for the length of a five-second check made the Status page's two-second poll queue up behind it, far enough to exhaust the web server's workers. The charts reuse the chart.js setup the CPU and memory graphs already use, rather than porting DB Doctor's own sparkline and latency chart, and they poll only while the database page is the one on screen and the tab is visible. Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com> Assisted-by: Claude Code:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
The database page reports what is wrong with the database configuration, but only to an admin who knows to open it. Settings > Overview is where they already look for things that need attention, so a failing check now says so there, linking straight to the findings. Running fifty-odd rules against a live server on every Overview load is not reasonable, so the outcome of a run is recorded in app config and the setup check reads that. Opening the database page records a fresh result, which means acting on a finding clears the warning without waiting for anything; a server nobody has looked at for a day re-runs the checks once and records that instead. The link is passed as a rich object parameter rather than through SetupResult's linkToDoc: the Overview does not render linkToDoc at all -- of the twenty-two checks on a stock install, the only one showing a link builds it the way this now does. Following that link needs the page to be addressable, so the admin settings page now reads its view from the URL fragment (#status, #database, leaves the back button alone while switching between them. Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com> Assisted-by: Claude Code:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Two problems the database page introduced. A rule whose formula cannot be evaluated was logged as a warning. That is not an exceptional condition: a variable that is not numeric on this particular server is ordinary — MariaDB reports concurrent_insert as AUTO where MySQL reports an integer — and it happens on every run. The effect was a warning written to nextcloud.log every time the page was opened, which then inflated the "Errors in the log" setup check on the same Overview page the database warning was added to. Verified on a MariaDB instance: three runs used to add three lines, and now add none. Nothing is lost by dropping the level. The rule already reports itself as skipped with the reason attached, which is where an admin should read it; the log line was only ever diagnostic detail. The apply-descriptor failure next to it stays a warning, because that one means a rule is written wrong rather than that the server is different. The other problem was two event listeners registered with anonymous handlers and never removed: visibilitychange in the live charts and hashchange on the settings page. Both are now named and taken off on unmount, which is what useLiveData already does. Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com> Assisted-by: Claude Code:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Twenty of the fifty-seven ported checks are removed, leaving thirty-seven. Nextcloud already reports missing indices, columns and primary keys on Settings > Overview, so repeating them here only gave an admin two places to read the same finding, and the transaction isolation check went with them. The query cache and MyISAM groups describe engines and features that a current Nextcloud does not use — the query cache was removed outright in MySQL 8.0 — so on a supported server they either skip or advise tuning something nothing touches. The rest are checks whose finding is not actionable: how long the server has been up, whether it has served enough queries to be worth analysing, slow_launch_time, skip_name_resolve, and the two binary-log ones that only matter with replication configured. Removing the schema-integrity rules pays for itself twice over. They were the only consumers of the code that dispatches AddMissingIndicesEvent and friends to every installed app and diffs the result against the live schema through SchemaWrapper, so that work no longer runs on a page load, and the app no longer reaches into OC\DB\SchemaWrapper -- one fewer private server class for a bundled app to depend on. The psalm baseline shrinks with it. It also removes the rule behind the MariaDB log noise fixed in the previous commit, so that fix now has nothing left to suppress on MariaDB. On a MariaDB test instance the page goes from 57 checks with 12 failing to 37 with 5, and the categories MyISAM, QueryCache, Security and Server disappear entirely. Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com> Assisted-by: Claude Code:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Member
|
Checking backport failure ⏳ |
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
skjnldsv
force-pushed
the
backport/1123/stable35
branch
from
September 9, 2026 19:20
66aad48 to
45576f6
Compare
skjnldsv
marked this pull request as ready for review
September 9, 2026 19:21
skjnldsv
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #1123
Warning, This backport's changes differ from the original and might be incomplete⚠️
Todo
Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.