Repository navigation
get_store_host: a slow size read fails alone and the polled read uses the stored size (#5718) - #5723
Conversation
| why the figure is missing instead of failing the whole card. */ | ||
| [cellText("Store size"), cellText(store.size_bytes == null | ||
| ? "unavailable" + (store.size_unavailable_reason ? " (" + store.size_unavailable_reason + ")" : "") | ||
| : fmtMb(bytesToMb(store.size_bytes)) + (store.size_age_seconds == null ? "" : " (hourly sweep, " + fmtMs(store.size_age_seconds * 1000) + " old)"))], |
There was a problem hiding this comment.
The age the card prints stops advancing after the first fetch.
size_age_seconds is a scalar the server computed once, at gather time, and it is baked into the payload. This page caches that payload and replays it on every poll tick without re-fetching — renderStoreHost at line 462:
const isPoll = !!(opts && opts.poll === true);
if (isPoll && lastStoreHostPayload) {
renderStoreHostPayload(box, lastStoreHostPayload); // no network call
return;
}So renderStoreHostPayload re-renders the same frozen integer on the 60 s poll, and only a first paint / hashchange / span change ever refreshes it.
Failure scenario: an operator opens Sweeps, the card reads Store size 225.0 GB (hourly sweep, 2.0 min old), and they leave the tab open. Sixty poll ticks later the stored figure is 62 minutes old, the service may have stopped sweeping entirely, and the card still reads 2.0 min old. The one number this PR adds to tell the reader how stale the size is is itself stale, without bound, and silently understates the age — which is the worse direction, because "2 minutes old" is the reading that makes an operator trust the byte count.
The payload already carries the fix: size_as_of is on the wire, and relTime is already in this file's import list and already parses the repo's naive-UTC stamps through parseUtc. Computing the age at render time ages correctly on the replay path:
| : fmtMb(bytesToMb(store.size_bytes)) + (store.size_age_seconds == null ? "" : " (hourly sweep, " + fmtMs(store.size_age_seconds * 1000) + " old)"))], | |
| : fmtMb(bytesToMb(store.size_bytes)) + (store.size_as_of == null ? "" : " (hourly sweep, " + relTime(store.size_as_of) + ")"))], |
(relTime renders "12m ago" / "2h ago" rather than "12.0 min old"; if the "old" wording matters, derive the seconds from Date.now() - parseUtc(store.size_as_of) at render time instead. Either way the derivation belongs in renderStoreHostPayload, not in the cached payload.) size_age_seconds is still right for MCP callers, where the response is read once.
| catch (PostgresException) | ||
| { | ||
| /* timescaledb_information.chunks does not exist on this connection's database -- no TimescaleDB | ||
| extension here (plain PostgreSQL BYO, or preloaded-but-not-CREATE-EXTENSION'd, same shape the | ||
| JobExecutionLoggingSql remarks describe for a different GUC). Zero/zero is the honest answer. */ | ||
| uncompressedBytes = 0; | ||
| uncompressedCount = 0; | ||
| } |
There was a problem hiding this comment.
Every non-timeout PostgresException here still becomes a measured "0 uncompressed chunks, 0.0% of RAM".
This PR gave the fact a null-with-reason vocabulary and then used it only for the timeout arm. Two blocks up, the size read's sibling catch now does the honest thing:
catch (PostgresException pg)
{
storeSizeBytes = null;
storeSizeAsOf = null;
storeSizeReason = $"this role cannot read it (SQLSTATE {pg.SqlState})";
}This one keeps the pre-#5718 reading — and the diff re-affirms it by writing the two zeros out explicitly. The comment justifies that for exactly one SQLSTATE ("timescaledb_information.chunks does not exist on this connection's database"), but the catch is unfiltered, so it swallows every other error code into the same answer.
Two reachable ones, neither of them "no TimescaleDB":
- 42P01 mid-statement, routinely.
UncompressedChunkSizeSqlwalks every uncompressed chunk and castsformat('%I.%I', chunk_schema, chunk_name)::regclassrow by row. A retention or compression policy dropping a chunk between the view read and that cast raisesrelation "_hyper_x_y_chunk" does not exist. On a store with hourly retention and compression jobs and hundreds of chunks, a whole-store walk racing a drop is ordinary, not exotic. - 42703 on a TimescaleDB upgrade.
.github/REVIEW-TRAPS.mdrecords the precedent atDarlingWorker.cs:10721: "Store self-metrics sweep fails on TimescaleDB 2.29+ (catalog chunk.schema_name/table_name removed); breaks the store-log sweep and collector-cost flush too; live today on the Linux compose image #3918 was exactly this for one statement (a catch-all census failing 42703 on TimescaleDB 2.29+)". A column rename intimescaledb_information.chunkswould make this read fail permanently and silently.
In both cases the wire says uncompressed_chunk_bytes: 0, uncompressed_chunk_count: 0, uncompressed_chunk_percent_of_ram: 0.0, uncompressed_unavailable_reason: null, and the web card prints 0 chunks, 0 MB (0.0% of RAM) — indistinguishable from a fully-compressed store. The #4211 figure is a RAM-pressure signal, so the fabricated zero reads as all clear in the one direction that matters.
This is the trap the repo already names: "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells "unknowable" as 0 ... End at END (NULL) and carry the NULL to the wire" (#3707), and "Likewise IsDBNull ? 0: an unmeasured pass is null with a *_measured flag beside it, not 0 MB" (#3682).
Narrow the zero to the SQLSTATE the comment actually argues for, and let the rest take the reason path the size read now has:
| catch (PostgresException) | |
| { | |
| /* timescaledb_information.chunks does not exist on this connection's database -- no TimescaleDB | |
| extension here (plain PostgreSQL BYO, or preloaded-but-not-CREATE-EXTENSION'd, same shape the | |
| JobExecutionLoggingSql remarks describe for a different GUC). Zero/zero is the honest answer. */ | |
| uncompressedBytes = 0; | |
| uncompressedCount = 0; | |
| } | |
| catch (PostgresException pg) when (pg.SqlState == PostgresErrorCodes.UndefinedTable | |
| && reads.UncompressedChunkSizeSql == UncompressedChunkSizeSql) | |
| { | |
| /* timescaledb_information.chunks does not exist on this connection's database -- no TimescaleDB | |
| extension here (plain PostgreSQL BYO, or preloaded-but-not-CREATE-EXTENSION'd, same shape the | |
| JobExecutionLoggingSql remarks describe for a different GUC). Zero/zero is the honest answer. */ | |
| uncompressedBytes = 0; | |
| uncompressedCount = 0; | |
| } | |
| catch (PostgresException pg) | |
| { | |
| /* #5718: anything else -- a chunk dropped by a retention/compression policy between the view read | |
| and the per-chunk regclass cast (42P01 mid-walk), a column drift on a TimescaleDB upgrade | |
| (42703, the #3918 shape), a refused grant -- is UNKNOWN, not zero. A fabricated 0 here reads as | |
| "nothing uncompressed, 0.0% of RAM", which is the #4211 signal saying all-clear. */ | |
| uncompressedBytes = null; | |
| uncompressedCount = null; | |
| uncompressedReason = $"this role cannot read it (SQLSTATE {pg.SqlState})"; | |
| } |
The when clause's second conjunct is only there because the 42P01 reading is a property of the shipped statement; if you would rather not couple to the seam, drop it and keep the SQLSTATE test alone. Either way the "a store without TimescaleDB still reports 0/0" claim in the PR body keeps holding — DarlingStoreHostProfileLiveTests line 170-171 still passes — and the reason wording stays the one DescribeReadTimeout's sibling already uses, so the two facts cannot word the same situation differently.
| /* #5718: the polled reads (this tool, and the web page that calls it every few minutes) take the | ||
| store size from the hourly sweep's stored figure and report its age, rather than paying | ||
| pg_database_size on every poll. --check-settings keeps the live read. */ | ||
| return await DarlingStoreHostProfile.GatherAsync( | ||
| postgresConfig, connection, token, useStoredStoreSize: true); |
There was a problem hiding this comment.
Non-blocking, but worth saying out loud: this flag reaches a third caller the PR body never names.
DarlingCloudIdentityProbeTests.GetStoreHostCallSite_OnlyInTheMcpToolTheWebDispatchAndTheDiagnosticsBundle pins three call sites, and the third is DiagnosticsBundleRunner.cs:493:
var host = DiagnosticsBundle.ParseReader(
await DarlingMcpStoreHostTools.GetStoreHost(postgres, config.Postgres, new StoreHostProfileCache(TimeSpan.FromMinutes(1)), ct));That is not a polled read — it is a one-shot verb an operator runs by hand, the same category as --check-settings, and the test's own comment says so ("read ... once, from a verb an operator runs by hand, as --check-settings does through GatherAsync"). The body's rule ("the polled reads use the stored size ... --check-settings keeps the live read") would put the bundle on the live side; the code silently puts it on the stored side, so a support bundle can now report a store size up to an hour old.
I don't think this needs reverting — the bundle embeds the whole JSON, so size_as_of / size_age_seconds / size_unavailable_reason travel with it, and a bundle that can't hang for 30 s on pg_database_size is arguably the better bundle. But it is a deliberate call that nothing in the PR records, and the one-minute cache it passes is now pointless for the size (the figure behind it only moves hourly). Please either state the choice in the body/changelog, or pass useStoredStoreSize: false through for the bundle.
| /// <see cref="GatherStoreFactsAsync"/> (see <c>SerialLoopStoreSizeSourceTests</c>, #3199). #5718: the | ||
| /// POLLED reads (<c>get_store_host</c> over MCP and the web page) no longer run this SQL at all — they take | ||
| /// the sweep's stored figure (<c>StoreSelfMetrics.LatestStoreSizeWithTimeSql</c>) and report its age, | ||
| /// because the web page polls every few minutes and this read measured 5.8 s mean / 30.7 s max on a slow | ||
| /// store; <c>--check-settings</c> is the only caller left, and it fails this one fact alone on a timeout.</summary> | ||
| public const string StoreSizeSql = "SELECT pg_database_size(current_database())"; |
There was a problem hiding this comment.
Non-blocking: this paragraph is right, and it leaves two derivations elsewhere describing the world it just changed.
1. ServiceCommandDeadlines.McpStoreHostProfileSeconds (ServiceCommandDeadlines.cs:483-491). Its derivation is spelled out and is now wrong for the MCP path:
GatherAsyncruns, sequentially, the version/buffer/size/uncompressed-chunk reads (GatherStoreFactsAsync, each individually bounded atCliStoreReadSeconds) ... Only two of those five statements are store-scaling (StoreSizeSql,UncompressedChunkSizeSql...), so the backstop clears TWO store-scaling statements at their own ceiling plus the same 5s margin
After this change the MCP path runs exactly one store-scaling statement, and (CliStoreReadSeconds * 2) + 5 is no longer the arithmetic of what it bounds. The constant's value is still a safe bound, so nothing breaks — but this repo treats those derivations as load-bearing rather than decorative, and the next reader sizing this will be reasoning from a premise the diff retired.
2. SerialLoopStoreSizeSourceTests.s_pgDatabaseSizeOwners's fourth-regime reason (SerialLoopStoreSizeSourceTests.cs:95-113) still says the entry covers "GatherStoreFactsAsync's only TWO callers ... --check-settings and the get_store_host MCP read below". That file's own doc says "The reasons are the assertion, not decoration — a third entry appearing means someone put a store-wide walk somewhere new, and the failure message hands them these two so they have to say which regime theirs is in." The census still passes (one literal, same file), but the reason now over-states who pays for the walk: only --check-settings does.
| /// so the same backward scan serves it. No parameters. | ||
| /// </summary> | ||
| public const string LatestStoreSizeWithTimeSql = $@" | ||
| SELECT total_bytes, metric_time | ||
| FROM collect.store_metrics | ||
| WHERE object_kind = '{StoreObjectKind}' | ||
| AND total_bytes IS NOT NULL | ||
| ORDER BY metric_time DESC | ||
| LIMIT 1"; |
There was a problem hiding this comment.
Non-blocking, and the pin in StoreHostSlowSizeTests.TheStoredSizeSql_IsTheRecordedSizeSqlWithTheTimeAdded does defuse the worst version of this — but the shape is one the traps file names:
Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy.
The predicate, the IS NOT NULL filter, the ordering and the LIMIT are now written twice, four lines apart, and kept honest by a test that string-replaces one into the other. That pin is asymmetric (a one-sided edit to either constant fails it), so it is better than the #3684 case — but it only exists because the copy exists. A shared tail makes the copy impossible and the pin unnecessary, and const string concatenation is a compile-time expression so both stay const:
/// <summary>The body both latest-store-size reads share — predicate, ordering and bound — so the two
/// projections below cannot drift apart (#5718; the #3684 "alias, do not copy" rule).</summary>
private const string LatestStoreSizeBodySql = $@"
FROM collect.store_metrics
WHERE object_kind = '{StoreObjectKind}'
AND total_bytes IS NOT NULL
ORDER BY metric_time DESC
LIMIT 1";
public const string LatestStoreSizeSql = "\nSELECT total_bytes" + LatestStoreSizeBodySql;
/// <summary>
/// <see cref="LatestStoreSizeSql"/> with the row's <c>metric_time</c> (naive UTC) added (#5718): the
/// polled <c>get_store_host</c> reads take the store size from the hourly sweep instead of paying
/// <c>pg_database_size</c> on every poll, and report how old the figure is. Same predicate and ordering,
/// so the same backward scan serves it. No parameters.
/// </summary>
public const string LatestStoreSizeWithTimeSql = "\nSELECT total_bytes, metric_time" + LatestStoreSizeBodySql;SerialLoopStoreSizeSourceTests.TheDiskCheckExecutesTheRecordedSizeConstant reads LatestStoreSizeSql at runtime (Assert.Contains("collect.store_metrics", …), "ORDER BY metric_time DESC", "LIMIT 1") and TheRecordedSizeReaderAndWriterCannotDisagreeAboutTheKind checks 'store' is in it — all still satisfied by the concatenated value. Only the new TheStoredSizeSql_… pin becomes redundant.
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Two blockers, each with a patch in an inline comment; the mechanism itself (per-fact containment, the stored-size switch, the error rewording, the CLI keeping the live read) reads correct and is well pinned.
sweeps.js:498— the age the card prints never advances after the first fetch.DarlingStoreHostProfile.cs:746-753— the uncompressed read still answers "0 chunks, 0.0% of RAM" for every non-timeoutPostgresException, while its sibling size read, in this same diff, learned to say why.
Claims
- VERIFIED — Timeouts matched on type and SQLSTATE, never message text; the caller's cancellation never swallowed —
DarlingStoreHostProfile.cs:605-607,DarlingAlertReadAdapter.cs:415-431, plus!cancellationToken.IsCancellationRequestedon both filters (:694,:740). - VERIFIED — The polled MCP/web reads take the stored size, run no
pg_database_size, and a refused grant degrades with a reason —DarlingMcpStoreHostTools.cs:113-116;DarlingStoreHostProfile.cs:674-683,:701-708. - VERIFIED —
size_age_secondsis computed as of the response —DarlingMcpStoreHostTools.cs:130-132sits outside thecache.GetOrGatherAsyncdelegate. True for MCP; see REFUTED for the web. - VERIFIED —
--check-settingskeeps the live read and gains the reasons in text and--json; a store without TimescaleDB still reports 0/0 —DarlingCliCommands.cs:948,:1137-1140, the format hunks,:746-753. - VERIFIED — The web map calls
GetStoreHost; Lite has no counterpart; the served head is byte-identical and the new prose rides the tail —DarlingWebEndpoints.cs:5790; empty Lite grep +DarlingMcpStoreHostTools.cs:31; every added sentence sits after<<GUIDE>>, and the canonical budget file recordsServed.Lengthonly (McpToolsListBudgetTests.cs:729-730). - VERIFIED —
FailureForkeeps the envelope and the nullable widening breaks no caller —McpHelpers.cs:783rendersex.Messagealone; the four positionalnew HostStoreFacts(...)sites still bind on the optional members. - UNVERIFIED — The production measurements (10.0-10.9 s failures, 22/day; 5.8 s mean / 30.7 s max; 11.8 s / 49.4 s) — field readings off a live slow store.
- UNVERIFIED — Npgsql 10.0.3's client-deadline exception shape, and that the connection stays reusable after it. If it instead breaks the connection,
BufferAndTempSql(:712) sits outside everytryand reintroduces the whole-profile failure. - UNVERIFIED — The 11 new tests, the revert check, and the suite counts (738 / 1,584 / 803 / 553 / 180) —
dotnet buildis unavailable here, and five of the elevenAssert.SkipWhenout withoutDARLING_TEST_PG. - REFUTED — "the web page now polls it every 2 to 3 minutes", as the justification for the switch —
sweeps.js:462-466replays the cached payload on poll ticks and re-fetches only on first paint, hashchange or span change. The page did re-fetch every 60 s while broken (the fetch errored, solastStoreHostPayloadwas never assigned and the guard never fired — that is the one-[ERROR]-per-poll symptom), but once the read succeeds the card stops calling the tool. The switch still stands on the MCP tool's own account; the same replay path is what breaks the new age field.
Riskiest lines
wwwroot/js/pages/sweeps.js:498— a server-computed scalar rendered on a path replayed from cache every 60 s, so the age freezes — UNPINNED (TheWebCard_ShowsTheSizeAge_…greps the file, it does not execute the render).DarlingStoreHostProfile.cs:746-753— an unfilteredcatch (PostgresException)answering 0/0 for every SQLSTATE — UNPINNED beyond the no-TimescaleDB case (DarlingStoreHostProfileLiveTests.cs:170-171asserts the 0/0 that makes the fabricated zero look right).DarlingStoreHostProfile.cs:674-683— the stored-size read and itsDateTimeKind.Unspecifiedstamp; the naive-UTC convention checks out (PgMigrations.cs:4422,DarlingWorker.cs:10718,DarlingMcpStoreHostTools.cs:198-202) — pinned byAPolledRead_TakesTheStoredSweepSize_…_LiveandAPolledReadWithNoStoredFigure_…_Live, both live-only.
Traps near this diff
- "
CASE WHEN ... ELSE 0 ENDspells "unknowable" as 0 ... End atEND(NULL) and carry the NULL to the wire." (#3707) — applies —:746-753;uncompressed_chunk_percent_of_ramfollows the fabricated zero to0.0, and the null-and-reason vocabulary sits three lines up at:740-745. - "Likewise
IsDBNull ? 0: an unmeasured pass is null with a*_measuredflag beside it, not 0 MB (#3682)." — applies — same site; the diff adds the flag and leaves it null on the arm that needs it. - "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies —
StoreSelfMetrics.cs:1201and:1215, kept in step by aReplace-based pin. Milder than #3684 (the pin is asymmetric), but a sharedconsttail removes the copy and the pin. - "Lite and Darling ship in parity ... a fix to one SKU's tool or description without the counterpart's is a finding" — does not apply — the Lite grep is empty;
DarlingMcpStoreHostTools.cs:31records the tool onKnownLiteMissingMcpTools. - "A JSON number above 2^53 is rounded by every JavaScript client (#3487)." / "
chunk_compression_statsreturns ZERO rows ... count fromtimescaledb_information.chunks(#3610)." / "a clamp is a silent lie about the window (#3699)." — do not apply — the new 64-bit fields are a byte count (~2.4e11) and an age in seconds; the read already counts fromtimescaledb_information.chunks(:587-592); the only clamp isMath.Max(0L, …)on an output age, not an input window, with writer and reader on one clock.
Divergence (blind read vs PR body)
- The web-poll premise and the age field collide. The body justifies the switch with a poll that
sweeps.js:462-466does not perform — and that same replay path is exactly what makessize_age_secondsgo stale on screen. "A cached profile still reports a true age" holds for MCP, not for the web card the sentence is about. - A third caller the body never names.
DiagnosticsBundleRunner.cs:493also moves off the live read, though the body's own rule puts one-shot operator verbs on the live side with--check-settings. - Two derivations left describing the old world.
ServiceCommandDeadlines.cs:483-491still derives the bound from "TWO store-scaling statements";SerialLoopStoreSizeSourceTests.s_pgDatabaseSizeOwnersstill names the MCP read as apg_database_sizepayer. Neither fails.
Everything else matched the blind read.
Reviewer checklist
- Build and run the suite — nothing here was compiled. Watch that
Assert.Equal(0, profile.Store.UncompressedChunkBytes)(DarlingStoreHostProfileLiveTests.cs:170-171) still infersT = long?, and thatAssert.Equal(1, await after.ExecuteScalarAsync(ct))(StoreHostSlowSizeTests.cs:166) binds against the boxedobject?. - Run the five live tests with
DARLING_TEST_PGagainst a cluster that may already holdobject_kind = 'store'rows —APolledRead_…assumes its probe row is the newest one. - Confirm Npgsql leaves the connection reusable after a client
CommandTimeout. - Load the Sweeps page against a real store and leave it open an hour — the card should age its "hourly sweep, N old" text, not freeze it.
- On a store that has never swept, confirm the card and the payload read "no hourly sweep has recorded the store size yet".
…ard age follows the clock (#5718) - The uncompressed-chunk read answers 0 chunks only when TimescaleDB is absent (42P01, 3F000, 42883); any other refusal is null with the SQLSTATE as its reason. - The web card derives the stored size's age from size_as_of at render time, so a replayed poll payload keeps ageing; Node-run test. - The support bundle stays on the stored size and says why in a comment; pinned. - Update the McpStoreHostProfileSeconds derivation and the pg_database_size census note; assert the MCP read is not a pg_database_size payer. - One shared tail for the two recorded-size statements, replacing the Replace pin.
Claude posting for Erik Darling. Addressed in 8d78abc: the chunk read keeps 0 only for a missing TimescaleDB and reports anything else as unknown with its SQLSTATE; the web card computes the age from size_as_of at render time; the bundle and the deadline derivation say they read the stored size.
Claude posting for Erik Darling.
Closes #5718.
Why
get_store_hostfailed every time on a test store, after 10.0 to 10.9 s (22 times in a day, an[ERROR]each time the page or an MCP client asked), with "Exception while reading from stream".DarlingStoreHostProfile.GatherStoreFactsAsyncran two store-scaling reads live on every call, each with a 10 s command timeout: the store size (pg_database_size) and the uncompressed-chunk total. On that store the hourly sweep's own versions of such reads measured: thestore_metricsinsert built onpg_database_size, mean 5.8 s / max 30.7 s over 60 calls; the per-hypertable size insert, mean 11.8 s / max 49.4 s. The code assumed a one-shot, user-invoked read, but the web card re-fetches it on every first paint, hashchange and span change (sweeps.jsreplays its cached payload on the 60 s poll ticks and does not re-fetch), and every MCP call is a fresh call. Each of those paid the slow live read and failed at 10 s while it was slow, and any one slow fact failed the whole profile.Which of the two live reads times out first was not measured (the issue says so too). The fix covers both.
What changes
Darling/PerformanceMonitor.Darling.Service/DarlingStoreHostProfile.cs,Mcp/DarlingMcpStoreHostTools.cs,wwwroot/js/pages/sweeps.js,PerformanceMonitor.Darling.Storage/StoreSelfMetrics.cs.timed out after 10 s); the version, buffer hit, temp bytes and settings still return. Timeouts are matched on type and SQLSTATE, never on message text:NpgsqlExceptionover aTimeoutException(what Npgsql 10.0.3 throws for a client deadline), orPostgresException57014 (a serverstatement_timeout). Cancellation from the caller's token is never caught (the filter also checks the token, and anOperationCanceledExceptionnever matches). A refused grant on the size read still degrades the same way, now with a reason (this role cannot read it (SQLSTATE ...)). The existingAlertReadRetrySeamTestsmeasurement says the connection stays open and reusable after a client deadline, which the settings read that follows depends on; a new test checks it.get_store_hostover MCP, and the web page (which calls the same method, see below), take the store size from the hourly sweep'scollect.store_metricsrow, through a newStoreSelfMetrics.LatestStoreSizeWithTimeSql(the same predicate and ordering asLatestStoreSizeSql, withmetric_timeadded; a test pins that equivalence). They no longer runpg_database_size. Thestoreobject gainssize_as_of(the row'smetric_time, naive UTC),size_age_seconds(as of the response, so a profile served from the 5 minute server cache still reports a true age; the web card does not use it, it derives the age fromsize_as_ofat render time, see review round 1), andsize_unavailable_reason(set when there is no stored row yet or the read failed; there is no fallback to the live read, which would bring the failure back).--check-settingskeeps the live read (GatherAsyncdefaultsuseStoredStoreSizeto false) and printssize unavailable: <reason>/uncompressed chunks: unavailable: <reason>when a fact is unavailable; its--jsongainsstoreSizeUnavailableReasonanduncompressedUnavailableReason.store_metricsholds a per-object total, compressed before/after bytes and a chunk count, none of which is "bytes in chunks not yet compressed". So this fact stays a live read on every path, with the per-fact failure from step 1. When it times out, or the role is refused it for any reason other than TimescaleDB being absent,uncompressed_chunk_bytes,uncompressed_chunk_countanduncompressed_chunk_percent_of_ramare null anduncompressed_unavailable_reasonsays why. (HostStoreFacts.UncompressedChunkBytes/Countbecame nullable for this; a store without TimescaleDB, meaning SQLSTATE 42P01, 3F000 or 42883, still reports 0/0; any other refusal reports null withthis role cannot read it (SQLSTATE ...).)pg_settingsread) now reaches the tool's catch asa store read timed out after 10 s (the store was slow to answer); retry in a moment(DarlingMcpStoreHostTools.FailureFor, passed throughMcpHelpers.FormatError, so the envelope shape and its census are unchanged) instead of "Exception while reading from stream". Anything else keeps its own message.["get_store_host"]entry callsDarlingMcpStoreHostTools.GetStoreHost(pinned by a test), so both outputs carry the same fields. The web card (sweeps.js) now showsStore sizeas<size> (hourly sweep, <age> old)and showsunavailable (<reason>)for either fact instead of a dash.grep -rn "StoreHost\|store_host" Lite --include=*.csfinds nothing. Darling only.The served tool description is unchanged (the tools-list byte budget only goes down). The reading guide (the text after
<<GUIDE>>, returned byget_tool_guide, outside that budget) now explains the stored size and itssize_as_of/size_age_seconds, the null-with-reason cases, the live uncompressed-chunk figures that fail alone on a timeout, and that--check-settingsreads the size live; pinned byMcpToolGuideHeadsPlatformTests.StoreHost_GuideTail_ExplainsTheStoredSizeAndTheAloneFailingFacts_OffTheServedHead. No existing pin held this guide text. Touched classes, every Census and Ratchet class (738 tests, 0 failed) and-trait Stage=Guard(1584, 0 failed) pass.Test plan
StoreHostSlowSizeLiveTests/StoreHostSlowSizeTests(11 tests) on a throwaway PostgreSQL 17 + TimescaleDB cluster (own folder, own port, stopped afterwards): a size read forced to time out withpg_sleepbehind the newStoreFactsReadsseam (1 s deadline) leaves the rest of the profile intact withtimed out after 1 s; the same for the uncompressed-chunk read, and the connection is still usable after it; a caller cancellation propagates asOperationCanceledException; a seededstorerow recorded 7 minutes earlier (a valuepg_database_sizecan never return) comes back fromGetStoreHostassize_bytes,size_as_ofandsize_age_secondsof about 420; no stored row gives the "no hourly sweep" reason; the escaped-timeout text; the classifier (accepts a deadline and 57014, refuses cancellation, a permission error and a connect failure); source pins for the MCP call, the web map, the CLI keeping the live read, and the web card.DarlingMcpStoreHostToolsLiveTests: it assertedsize_byteswas non-null, which now needs a stored sweep row; it seeds one (and deletes it) and also assertssize_as_of/size_age_secondsunder themcpandviewerroles.get_store_hostor the store-size source (includingMcpToolsListBudgetTests,McpPayloadContractCensusTests,SerialLoopStoreSizeSourceTests,StoreHostProfileCacheTests,StartupHostProfileLogTests,DarlingStoreHostProfile*,DiagnosticsBundle*), 0 failed; 553 across all Census, Ratchet, pin and command-timeout classes (SlowClassCensusTestsalone: 7 of 7);-trait Stage=Guard: 1,584 of 1,584. After mergingorigin/dev, the store-host, census and budget classes again: 180 run, 0 failed.Installer.Tests(CI decides). Not run: a real slow store; the 10 s boundary is exercised with a 1 s deadline.For the reviewer: a first run of the "uncompressed read times out" test failed once because its default
pg_database_sizeread exceeded the 1 s test deadline on a cold cluster; the test now stubs the other slow read, so each test exercises one fact.Changelog
SECTION: Fixed
ENTRY:
get_store_hostno longer fails when the store's size reads are slow ([get_store_host fails after 10 s when the store's live size reads are slow #5718]) - On a store whose size reads were slow,get_store_host(MCP and the web dashboard's store host card, which re-fetches it on each page load) failed every time after 10 s with "Exception while reading from stream", because the live store size and the live uncompressed-chunk total each had a 10 s limit and either one failing failed the whole profile. A slow size or uncompressed-chunk read now makes only that fact unavailable, with the reason (for example "timed out after 10 s"), and the rest of the profile returns. The MCP tool and the web card now take the store size from the hourly sweep's stored figure and report its age (size_as_of,size_age_seconds) instead of reading it live on every call; the uncompressed-chunk total is still read live because the sweep does not store it.--check-settingskeeps the live size read and now prints the reason when a fact is unavailable. A timeout that still fails the whole read says it timed out and after how long.REF:
[get_store_host fails after 10 s when the store's live size reads are slow #5718]: get_store_host fails after 10 s when the store's live size reads are slow #5718
Support bundle caller
DiagnosticsBundleRunner.StoreSectionAsyncalso callsGetStoreHost, so the bundle's store host section now carries the stored size too. It stays on the stored figure on purpose: a support bundle must not wait on a slow store, and the livepg_database_sizewalk is the read that measured 5.8 s mean / 30.7 s max. The section is the tool's whole payload, so it includessize_as_of,size_age_seconds,size_unavailable_reasonanduncompressed_unavailable_reason, which tell the reader how old the number is or why it is missing.--check-settingsis the only live caller. A test pins both facts.Review round 1
All in commit
8d78abc81(one commit, pushed to this branch).IsTimescaleAbsent) keep the 0/0 answer. Every otherPostgresExceptiongives null bytes and count withuncompressed_unavailable_reason=this role cannot read it (SQLSTATE nnnnn), the timeout arm's shape.uncompressed_chunk_percent_of_ramwas already computed only from a non-null byte count, so it is null with them. The tool guide sentence now says timeout or refusal. Tests: live theory for a missing view and a missing schema (both 42P01, answer 0/0), live theory for a raised 42501 and a division by zero (null + reason, connection still usable, size and buffer facts intact), and a pure theory on the classifier.sweeps.jsnow hasstoreSizeAgeSeconds/storeSizeText, and the card cell callsstoreSizeText(store, Date.now()): the age comes fromsize_as_of(viaparseUtc, naive UTC) at render time, withsize_age_secondsonly as a fallback whensize_as_ofis absent. A replayed payload therefore ages. Test: a Node harness (store-host-size-age-harness.mjs, the repo's existing pattern) runs the shippedsweeps.jsand shows one payload as 10.0 min, 11.0 min and 1.2 h old at three render times; Node-less machines skip it, and a source pin also checks the card derives the age fromsize_as_of. One trade-off: the age is now measured against the browser's clock, so a browser clock behind the store's reads 0 rather than negative.size_as_ofand both reasons, now pinned by a test and explained in a comment.McpStoreHostProfileSecondscomment no longer says "TWO store-scaling statements" for the MCP path (it names the one store-scaling statement left and why the bound keeps two ceilings; the value is unchanged). The MCP read's wording ins_pgDatabaseSizeOwnersnow says only--check-settingspays the live size, and a newTheMcpStoreHostRead_IsNotAPgDatabaseSizePayerasserts the tool file has nopg_database_size, noStoreSizeSql, and passesuseStoredStoreSize: true. (The file entry itself stays, becauseDarlingStoreHostProfile.csstill owns the literal.) I also reworded a tool comment that spelledpg_database_sizeso that assertion could be exact.StoreSelfMetrics.LatestStoreSizeSqlandLatestStoreSizeWithTimeSqlare now built from one privateLatestStoreSizeTailconst. TheReplace-based pin is replaced by one that checks the two statements end in the same text and that the tail appears once in the source. The resulting SQL text is unchanged.Tests run (rig: throwaway PostgreSQL with TimescaleDB, stopped):
StoreHostSlowSizeTests+StoreHostSlowSizeLiveTests+SerialLoopStoreSizeSourceTests46/46 (no skips); every*Census*and*Ratchet*class plus the touched and related classes 911 run, 0 failed, 1 skipped (a role-provisioning live test needingDARLING_TEST_PGRUNTIME);-trait Stage=Guard1584 run, 1 failed on the first pass (TestsNoSilentPassGuardTests: my Node test returned from a catch block), fixed and re-run green; theLiveClockCensusclass also failed once because my new test comment named the chunk catalog view, fixed by rewording. Not run: the full suite, and the live test that edits the rig's own conf (GatherAsync_ProducesEveryVerdict_EndToEnd_Gated, needsDARLING_TEST_PGRUNTIME).