Repository navigation
get_object_locking says WHEN its snapshot was collected, on both SKUs (#3880) - #3883
Merged
Merged
Conversation
…#3880) Erik's ruling on the judgment call #3878 recorded and #3879 shipped: stamp it, do not roster it. #3879 gave Darling's four object-locking reads the server's newest capture as their anchor, replacing a per-database_name MAX group that made every name the store had ever seen immortal (#3876 was the field report; #3877 fixed Lite's identical port). A side effect of that fix was that the read became VISIBLE to the latest-anchored-read census in McpPayloadContractCensusTests for the first time: the census ignores an anchor below parenthesis depth zero, because such an anchor picks a SET rather than a snapshot — so the defect itself had been hiding the read from the very rule it broke. The census asked its standing question, "a latest-anchored read projects its anchor column so the tool can stamp the snapshot", and #3879's lane answered by rostering the constant as pre-existing debt beside the two IndexUsage reads. Erik's ruling takes the better end state: a read that now resolves ONE instant can say which instant. So both get_object_locking bodies publish captured_at — the one spelling #3637 settled on for a latest read's stamp — taken from the read's own anchor column, projected on the ROW statement. Never a second MAX(collection_time) query for the stamp: that one can resolve to the NEXT capture landing between the two reads, and stamping rows from capture N with capture N+1's time is the failure the per-read rule in McpLatestSnapshotStampTests exists to prevent. Every row of this read comes from one capture, so rows[0] is the stamp for all of them, which is exactly how get_database_sizes one screen down has carried its own. Both SKUs in one commit because the two tool bodies mirror each other field-for-field and Lite carries the same convention in its own latest-read roster — an asymmetry here would be a new one, not a respected one. The descriptions and both instructions tables say LATEST IS A TIME in the stamped siblings' words, and they say the thing that makes this read's stamp matter more than most: object stats are collected DAILY, so on a perfectly healthy server the contention picture an agent reads is routinely hours old, and without a stamp it reads as "now". The Lite row type gains CollectionTime; the Locking grid does not bind it. The Viewer's own IndexLockingRow and its three reads are untouched — they are a display surface with a clock of their own, not an MCP payload.
…he shrink-only census shrinks back, and the live pin asserts the stamp against a real store (#3880) McpLatestSnapshotStampTests moves GetIndexLockingAsync from the roster exemption to the stamped set on both SKUs' suites, and the entry #3879 added to LatestAnchoredReadsWithoutTheirStamp is removed - the census resumes the only direction it was built to move. The surface pin asserts the stamp rides the get_object_locking envelope, and the #3879 live rename pin now also asserts the stamp is present and equals the newest capture's own collection_time (projected on the row statement, never re-read with a second MAX that could stamp the NEXT capture - the one-line doc dedup rides along). Erik's ruling on #3878's flagged roster call, executed: a reader should not have to open a census roster to learn how old an answer is.
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.
Erik's ruling on #3878's flagged call, executed
PR #3879 made
DarlingObjectStatsReader.IndexLockingSqlvisible to the latest-anchored-reads censusfor the first time — its old per-name anchor sat inside a CTE, which the census's scanner ignores as
picking a set, so the defect itself had been hiding the read. That lane rostered the newly-visible
read on the shrink-only
LatestAnchoredReadsWithoutTheirStampexemption list, argued it aspre-existing debt made visible rather than new debt, and flagged the call for Erik. The ruling:
stamp it, don't roster it — a reader should not have to open a census roster to learn how old an
answer is.
The stamp
Both SKUs'
get_object_lockingpayloads now carry the snapshot's owncollection_time, the samefield and semantics the stamped snapshot reads beside them already carry. Two details on purpose:
MAX(collection_time)issued at stamptime could stamp the next capture if a collection lands between the row read and the stamp read;
the stamp is the anchored rows' own
collection_time, so it is the snapshot's timestamp byconstruction.
(
Lite.Tests/McpLatestSnapshotStampTests), so parity is a stamped read on each side, not a Darlingspecial.
The census moves the right way again
The
IndexLockingSqlentry #3879 added toLatestAnchoredReadsWithoutTheirStampis removed — theshrink-only census resumes the only direction it was built to move.
McpLatestSnapshotStampTeststakes the read through its stamped arm on both SKUs' suites, the surface pin asserts the stamp
rides the tool envelope, and #3879's live rename pin now also asserts the stamp is present and equals
the newest capture's own time — verified against a real Timescale store before this left the branch
(1/1 live, container removed).
Verified locally: Darling.Tests and Lite.Tests build 0/0;
McpLatestSnapshotStampTests35/35,McpPayloadContractCensusTests67/67,DarlingMcpObjectStatsToolsSurfaceAndSqlTests16/16,ViewerFinOpsSqlTests49/49,DarlingIndexLockingRenamedDatabaseLivePostgresTests1/1 (live).Lite.Tests execution is CI's, as ever.
Follow-up to #3876 → #3877 and #3878 → #3879. Closes #3880.