Skip to content

Hold alert threshold overrides for a server tag (#6926) - #6939

Merged
erikdarlingdata merged 4 commits into
devfrom
feature/6926-tag-threshold-overrides
Oct 30, 2026
Merged

erikdarlingdata merged 4 commits into
devfrom
feature/6926-tag-threshold-overrides

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Claude posting for Erik Darling.

Closes #6926.

Draft on purpose: this must not merge before the current release is published.

Why

Overrides (#5933) were per server or per database. Production and development needed different thresholds, which meant one override per key per server, and a server added to a tag later started on the global values. A tag scope fixes both.

What changes

  • Store, V227 (PgMigrations.cs, StorageVersion 227): config_threshold_overrides gains nullable scope_tag_id, server_id becomes nullable, a CHECK says exactly one of the two is set (existing rows have a server, so they stay valid), a second CHECK says a tag row has no database, and a partial unique index keeps one value per tag and key. No foreign key on the tag, as V181 did for routes, so a deleted tag leaves the row as an orphan. All statements are idempotent. The viewer's schema probe has a V227 sentinel, MapProbedSchemaVersion has its arm, and RungRewindArtifacts has the V227 rewind. docs/darling/admin.md has the V227 rolling-back line: an older build's loader stops on a row with no server, so it applies no override until tag rows are removed.
  • Grants: no new object or function, so provision-roles.sql and DarlingManagedRoles need no new line. The V191 table-level grants (admin and mcp write, viewer read) already cover the new column.
  • Tag rule: TagThresholdResolver (Storage) runs NotificationRouteScopeExpander.Expand, the routes' own code, on one synthetic route per tag. A tag covers its servers and every tag below it, a deleted tag is orphaned and covers nothing, unreadable tag tables cover nothing. The deepest tag wins (the routes' ScopeTagDepth). The routes break a same-depth tie on the lowest route id, so an override breaks it on the lowest row id (the oldest override).
  • Precedence (DarlingThresholdOverrides): database, server, tag, global. Tag winners are resolved per server at load, so ResolveWire stays a dictionary lookup. Every evaluator reads through ResolveWire (the alert engine, the PostgreSQL count checks, the two worker call sites), so all of them see tag overrides.
  • MCP: set_alert_threshold_override takes tag (name or id) in place of server_name, clear works the same. Writes arm only (an investigation request's change is for its server only), no database_name. A name two tags share is refused until the id is used. An override whose tag was deleted can still be cleared by id. get_alert_settings with server_name reports source: tag_override with the tag and tag_id, and lists tag_overrides.
  • Web Admin tab: tag rows, orphaned rows marked, a Tag select in the form, Edit and Remove for tag rows. Viewer: the Threshold Overrides window lists tag rows (orphaned ones marked) and its scope box offers tags.

Test plan

  • Darling.Tests.exe -class "*ThresholdOverrideTagScopeTests*" on a throwaway PostgreSQL cluster: Total 9, Failed 0. Covers precedence, nested tags, the same-depth tie, an orphaned tag and unreadable tag tables matching nothing, the MCP set/clear/get round trip, misuse refusals, orphan list and clear, and V227 on a store that already held server overrides (rows kept, constraints, second run changes nothing).
  • dotnet build Darling/Darling.Tests : 0 errors.
  • Neighbouring classes (ThresholdOverride*, AlertThresholdOverride*, AgentLevel2Store, tool-undo, MCP payload census): Total 169 across ThresholdOverrideTests, ThresholdOverrideLiveTests, ThresholdOverrideWebLiveTests, ThresholdOverrideGridTests, AlertThresholdOverrideTests, ViewerSchemaVersion*, AgentLevel2StoreTests, AgentChangeToolUndoLiveTests, ToolUndoAnswersLiveTests, McpPayloadContractCensusTests, Failed 0.
  • CI fixes after the first run: V227 declared in the data-moving rung census (MigrationDataMovingRungCensusPins) with SetsTheFloor false, since it only touches the small operator-written overrides table, so MigrationLockWaitTimeoutSeconds is unchanged. The tag-name read in DarlingMcpThresholdOverrideTools now uses postgres.CreateCommand (the store receiver McpReadCommandTimeoutTests recognises). Census, Ratchet, Guard, Migration, Rung, SchemaVersion, McpReadCommandTimeout and threshold override classes: Total 483 and 753, Failed 0.
  • Not run: the full suite (CI runs it), a click-through of the Viewer window. The web page and Viewer changes are covered by the build and the existing grid tests only.

Notes

  • Agent arm: a tag override is refused with tag_writes_arm_only, because the investigation arm's change is scoped to the request's server and its journal and undo key on a server id.
  • Considered, not added: a tag-row journal entry for the agent arm, since the arm refuses tags.

Click-through (web Admin, Threshold Overrides)

Built the service from this branch and ran it on a throwaway PostgreSQL cluster with its own config file (first log line named that file). Three servers under two tags (prod holds two servers, staging one). Driven with a headless browser at 1400 wide and at 390 wide.

  • Add a tag override (tag prod, cpu.threshold_percent, 70): row shows Tag: prod, saved, no server needed.
  • Edit it: the Edit button loads the tag, key and value into the form, saved as 75, the row shows 75 with the same scope.
  • Clear it: Remove answered "Override removed. The global value applies again." and the row went away. get_alert_settings for a prod server then read source: global again.
  • Source: with the prod override at 75, a prod server read effective_value 75, source tag_override, tag prod, a server outside the tag read global. A server-level override of 90 on one prod server read effective_value 90, source server_override, so the server value still wins over its tag.
  • Phone width: the form stacks, the table scrolls sideways like the other admin tables, nothing clipped.
  • Found nothing to fix on the page. One setup note: a server-level override cannot be saved for a server the service has not connected to (the page says so clearly), so the server-level row for the precedence check was seeded straight into the store.

Also fixed: CA1068 in ViewerDataService.CollectionHealth.cs. GetSweepPressureAsync now takes the cancellation token last, and its two callers in ViewerSweepPressureLiveTests pass it by name. The tag scope test used Assert.Equal(1, ...Count), which raised xUnit2013, now Assert.Single.

Run: ThresholdOverrideTagScopeTests, ViewerSweepPressureLiveTests, ViewerSchemaVersionGateTests on the throwaway cluster: Total 44, Failed 0.

Changes after a code check

  • A failed read of the tag tables no longer drops every tag override. DarlingThresholdOverrides.LoadAsync threw nothing when ReadRouteScopeInputsAsync swallowed a tag read error, so a snapshot with no tag winners was installed and stayed until the next config version bump. It now throws when the tag tables could not be read, so the worker keeps the whole snapshot in force, the same as a failed read of the overrides table. Trigger: a reload that coincides with a connection reset or statement timeout on the server_tags or server_tag_map read.
  • get_alert_settings now filters tag rows through the loader's own accept rule (DarlingThresholdOverrides.Accept, made internal) instead of its own known-key and integer checks. Before, a deeper tag whose value was outside the key's bounds (direct SQL, or a bound tightened later) could be named in the view while the engine skipped it and applied a shallower tag.
  • Tests in ThresholdOverrideTagScopeTests: a live test where the tag map cannot be read (the loader throws, the previous tag and server values stay), and a live test where the deeper tag holds 5000 for a percent key (the loader and the view both name the shallower tag at 85).
  • Ran against a throwaway PostgreSQL cluster: ThresholdOverrideTagScopeTests Total 11, Failed 0. All threshold override classes (*ThresholdOverride*) ran Total 90 with 1 failure, a wrong count in my new test, fixed and re-run in the class above. Not re-run after the final rebase onto the branch.

A threshold override can now be set for a server tag. Precedence is database, then server, then the deepest tag the server is under (the notification routes' own rule), then global. Adds V227, the MCP tag argument, the get_alert_settings tag source, the Viewer and web Admin tag rows, and tests.
… rule in the server view (#6926)

LoadAsync now throws when the tag tables could not be read, so the worker keeps the whole snapshot in force instead of installing one with no tag winners until the next config change.

get_alert_settings filtered tag rows through its own known-key and integer checks. It now calls the loader's Accept, so a deeper tag whose value is outside the key's bounds is skipped there as it is in the engine, and the view names the tag the engine applies.
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 30, 2026 00:03
…e read from the store source (#6926)

V227 adds a nullable column, two CHECKs and a partial unique index to the
operator-written config.config_threshold_overrides table, a handful of rows, so
it is declared SetsTheFloor false and the lock-wait multiple stays 5.

The tag-name read in the override tool now uses postgres.CreateCommand like its
sibling reads, which is the store receiver the read-deadline census recognises.
@erikdarlingdata
erikdarlingdata merged commit 6fbf2e0 into dev Oct 30, 2026
25 of 26 checks passed
@erikdarlingdata
erikdarlingdata deleted the feature/6926-tag-threshold-overrides branch October 30, 2026 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant