fix(dash): let the invalid-config banner clear itself once the config is fixed - #185
Merged
Merged
Conversation
… is fixed The banner above the headline reads present-tense — "Open Settings, fix the configuration, and save it again" — but it was driven by a count over the FILTER'S WINDOW, and the dashboard's default view has no window at all. So it reported an incident forever after it was resolved. On this deployment that is exactly what happened. All 1,752 invalid-config requests fall between 2026-08-31 17:56 and 2026-09-01 02:07 — a single afternoon, caused by a config key removed with no migration for the accounts already using it, and fixed since. Zero in the last 24 hours. Yet every viewer of the default view was still being told, days later, to go and repair a configuration that was already correct. An alarm that cannot switch itself off is one people learn to scroll past, which costs precisely what it was built to buy — and it was built after a real incident where nine accounts' compaction silently went to zero for hours. So the banner now reads InvalidConfigRecent: the same fact over the last hour, scoped to the account and to nothing else. An hour because a config that cannot build affects EVERY request for that account while it lasts, so the signal appears within seconds on any account with traffic — long enough that a quiet account still trips it, short enough that it clears on its own once someone fixes the config, without anyone needing to know that clearing it is a thing that needs doing. Facets are ignored as well as the time range, deliberately: filtering to one model must not be able to hide a live breakage showing up on another. InvalidConfigRequests is unchanged and still reported — the historical fact is worth having, it just is not a call to action. The new test asserts both halves: resolved history raises nothing, and a breakage happening now still raises it. Verified to fail if the recency term is dropped. Signed-off-by: Osher Elhadad <Osher.Elhadad@ibm.com>
This was referenced Sep 3, 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
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.
The red banner above the headline reads present-tense — "Open Settings, fix the
configuration, and save it again" — but it was driven by a count over the
filter's window, and the dashboard's default view has no window at all. So it
kept reporting an incident forever after it was resolved.
On this deployment that is exactly what happened. All 1,752 invalid-config
requests fall between
2026-08-31 17:56and2026-09-01 02:07— a singleafternoon, caused by a config key removed with no migration for the accounts
already using it, and fixed since. Zero in the last 24 hours. Yet every viewer
of the default view was still being told, days later, to go and repair a
configuration that was already correct. That is what prompted the report.
An alarm that cannot switch itself off is one people learn to scroll past, which
costs precisely what it was built to buy — and it was built after a real incident
where nine accounts' compaction silently went to zero for hours before anyone
noticed, from a log line rather than from this page.
The change
The banner now reads a new
InvalidConfigRecent: the same fact over the lasthour, scoped to the account and to nothing else.
account while it lasts, so the signal appears within seconds on any account with
traffic. Long enough that a quiet account still trips it; short enough that it
clears on its own once someone fixes the config, without anyone needing to know
that clearing it is a thing that needs doing.
model must not be able to hide a live breakage that happens to be showing up on
another. The tenant term is kept, so no account sees another's problem.
InvalidConfigRequestsis unchanged and still reported. The historical fact isworth having; it just is not a call to action.
Deliberately not "delete the banner"
The request was to remove the message, and removing it outright was the smaller
diff. I did not, because the message is correct when the condition is live, and
deleting it would re-open the exact blind spot that cost hours during the incident
that motivated it. Making it self-clearing gets the same result for the reader —
no banner, because nothing is broken — while leaving the alarm able to fire next
time. If you would rather it were gone entirely, say so and I will remove it.
Tests
TestInvalidConfigRecentIgnoresResolvedHistoryasserts both halves: resolvedhistory raises nothing, and a breakage happening now still raises it. Verified to
fail when the recency term is dropped, so it is not a test that only passes.
go test ./dash/ ./proxy/green;go vetandgofmtclean.Deployed and confirmed against the live database:
invalid_config_requests1752,invalid_config_recent0 — banner gone.Second commit: the KV-cache and keep-alive reads had the same bound problem
Caught in production by the 503/504 watch rather than by a test.
/api/kvcache/simulatereturned 503 to a real user, and that morning's log carries 11 successes against 6
timeouts on that route, plus timeouts on
/api/kvcache,/api/kvcache/rowsand/api/keepalive.Intermittent-by-load is the signature of a bound set too close to the work, not of work
that is unbounded — and the measurements say exactly that. On the production database
through the real handlers, on an idle box with no contention:
/api/kvcache/simulate/api/kvcache/api/kvcache/suggest/api/keepalive/behaviour/api/keepalive/recommend/api/kvcache/rows/api/keepaliveEvery one completes. They fail only once a second reader, live traffic, or a janitor pass
exists — the normal state of a shared dashboard.
They move to
dashHeavyTimeoutas a family, not pruned to the four seen failing: theyread the same rows through the same shapes, so the ones that have not timed out yet are the
ones nobody has opened under load.
Deliberately not cached, unlike the aggregates that got the same bound: these finish in
seconds, so the bound alone stops the 503s, and caching nine more routes means nine more
keys to get right for a problem the bound already solves. If they get slower, caching is the
next move, not a longer bound.
Two guards on routeBounds itself
It is keyed by pattern string, so a typo is not a compile error — it is an entry that
matches nothing and silently leaves the route on the default, which is the very
"timeout that looks configured and is not" the map was introduced to eliminate.
Both verified to fail: one typo'd key trips both at once.
12 routes remain on the 10s default and each was measured cheap —
/api/series0.06s,/api/requests10ms,/api/breakdown0.6s,/api/sessions1.5s,/api/capture~0s, pluswhoami/config/archive/benchmarks. The default now means "this route is actually cheap"
rather than "nobody thought about this one".