Repository navigation
fix(data): a per-node hub's init timeout says which wait inside its one leg was outstanding - #5312
Conversation
…ne leg was outstanding (refs #1122) The attributed DataContext time-box names the pending type-source leg, and its doc claimed that separates "a storage read never came back" from "a remote hub never answered". For a per-node hub it does not: the one leg, …/MeshNode, is MeshNodeTypeSource.Initialize, which concatenates an unbounded durable read ahead of the routing-supplied own-node stream. The first attributed production sample (Collaboration, memex-cloud, 2026-09-21 17:06Z) could therefore be either. - IReportsInitialLoadProgress: optional, diagnostic-only capability of a type source. - TypeSourceInitializationLedger keeps the type source per pending key and appends its sentence. - MeshNodeTypeSource records seed-read start/settle (row / no row / fault) and routing emissions, and reports which half is outstanding. No bound changed, nothing waits on the new state. Control: with the ledger printing keys only, DataContextInitTimeoutNamesTheWaitInsideTheLegTest is red; with the change 5/5 in the attribution suite, 4/4 progress sentences, 11/11 MeshNodeTypeSource tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 0) 1 files 1 suites 2m 41s ⏱️ Results for commit ad5d0a4. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved findings remain in failure-path safety, initialization wiring and state publication, fallback and timing handling, and documentation/test coverage.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds diagnostic-only progress reporting for per-node hub initialization timeouts, distinguishing durable storage waits from routing-stream waits.
Changes:
- Adds optional progress reporting and ledger integration.
- Instruments
MeshNodeTypeSourceinitialization. - Adds tests and documentation for diagnostic messages.
| File | Summary and final review comments |
|---|---|
test/MeshWeaver.Graph.Test/MeshNodeTypeSourceInitialLoadProgressTest.cs |
Tests progress descriptions; no final comments. |
test/MeshWeaver.Data.Test/DataContextInitTimeoutNamesTheWaitInsideTheLegTest.cs |
Tests ledger rendering; no final comments. |
src/MeshWeaver.Graph/MeshNodeTypeSource.cs |
Tracks seed and routing progress. Moderate findings: add Initialize wiring coverage (3 votes), publish settlement state atomically (3 votes), instrument the no-routing fallback (1 vote), and use monotonic elapsed-time measurement (1 vote). |
src/MeshWeaver.Documentation/Data/Architecture/DataContextInitializationTimeout.md |
Documents diagnostic messages. Nit (1 vote): describe both sequential waits in the overview. Moderate (1 vote): cover the documented FAULTED variant with a test or revise the claim. |
src/MeshWeaver.Data/TypeSourceInitializationLedger.cs |
Appends progress details to pending legs. Critical (1 vote): protect the initialization failure path from exceptions in optional progress reporters. |
src/MeshWeaver.Data/IReportsInitialLoadProgress.cs |
Defines the diagnostic capability; no final comments. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| internal IReadOnlyCollection<string> Pending => pending | ||
| .Select(kv => kv.Value is IReportsInitialLoadProgress progress | ||
| && progress.DescribeInitialLoadProgress() is { Length: > 0 } detail | ||
| ? $"{kv.Key} [{detail}]" | ||
| : kv.Key) |
There was a problem hiding this comment.
Real, and measured — fixed in ad5d0a4. TypeSourceInitializationLedger.DescribeProgress guards the reporter call and prints the fault in place of the sentence ([progress report FAULTED (InvalidOperationException: reporter broke)]), so it is surfaced in the message the operator reads, not hidden. New case in DataContextInitTimeoutNamesTheWaitInsideTheLegTest with a throwing reporter: with the guard removed the hub wedges (the probe times out behind a shut gate — the test's own wedge assertion fails); with it, the hub reaches FAILED and the fault is named. 5/5.
| .Concat(_ownNodeStream | ||
| .Do(_ => _initialLoad.RoutingEmitted()) | ||
| .Select(n => (Node: n, Durable: false))) |
There was a problem hiding this comment.
Agreed — fixed in ad5d0a4. The wiring now lives in two helpers on InitialLoadProgress — TrackSeed(read) (Defer → record start → Take(1) → record value / fault / empty completion) and TrackRouting(stream) — and Initialize/DurableSeed compose them with no other recording code. TrackedSeedAndRouting_RecordAtSubscriptionSettlementAndEmission drives them with Subjects: nothing recorded at composition, "outstanding" after subscription, settled on the read's value, routing counted on emission; TrackedSeed_Fault_IsRecordedAsTheOutcome covers the fault arm. What is still read-only is that Initialize calls these two helpers — two lines.
| if (Interlocked.CompareExchange(ref seedSettledTicks, DateTimeOffset.UtcNow.UtcTicks, 0) == 0) | ||
| Volatile.Write(ref seedOutcome, outcome); |
There was a problem hiding this comment.
Correct — fixed in ad5d0a4. Seed state is now one immutable SeedState(StartedTicks, SettledTicks, Outcome) record published by a single reference swap (Volatile.Write on start, Interlocked.CompareExchange of a whole new record on settle, first settle wins), and Describe reads the reference once. A concurrent reader sees either the whole settlement or none of it.
Test Results (shard 3)429 tests 429 ✅ 53s ⏱️ Results for commit ad5d0a4. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)537 tests 537 ✅ 1m 14s ⏱️ Results for commit ad5d0a4. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 2 files 2 suites 6m 19s ⏱️ Results for commit ad5d0a4. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)1 579 tests 1 579 ✅ 9m 14s ⏱️ Results for commit ad5d0a4. ♻️ This comment has been updated with latest results. |
…a throwing reporter cannot wedge the failure path (refs #1122) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 5) 5 files 5 suites 14m 20s ⏱️ Results for commit ad5d0a4. |
Test Results 13 files 13 suites 34m 43s ⏱️ Results for commit ad5d0a4. |
Refs #1122 — does not close it (
sev:Hcloses on post-roll verification; see below).The gap
The attributed
DataContexttime-box names the pending type-source leg, andDoc/Architecture/DataContextInitializationTimeoutsaid that leg is what separates "a storage read never came back" from "a remote hub never answered". For a per-node hub it does not. Its one leg,…/MeshNode, isMeshNodeTypeSource.Initialize:So the newest attributed sample on #1122 —
Collaboration, memex-cloud, 2026-09-21 17:06:24Z, "type-source legs still outstanding: 7j8ehN2m0UCo51iBcrGL2A/MeshNode" — is consistent with either a read that never returned (thepg-readgate) or a routing stream that never delivered. Those are opposite investigations.The change (instrument only — no bound, no retry, nothing waits on it)
IReportsInitialLoadProgress(new,MeshWeaver.Data): optional, diagnostic-only capability of a type source.TypeSourceInitializationLedgerkeeps the type source per pending key and appends[sentence]when it implements the capability.MeshNodeTypeSourcerecords seed-read start / settle (row, no row, fault) and routing emissions, and reports which half is outstanding: "durable seed read of 'P' outstanding for 115.0s — a storage read that has not come back" vs "… found no row after 0.1s; the routing-supplied own-node stream has not emitted" vs "… emitted N time(s) and none was accepted". Numbers are formatted invariant.Controls
DataContextInitTimeoutNamesTheWaitInsideTheLegTest: with the ledger printing keys only → failed (message carries the leg, not the sentence); with the change → passes. WholeDataContextInitTimeout*family 5/5.MeshNodeTypeSourceInitialLoadProgressTest4/4 (the four sentences; first settle wins overTake(1)'s completion).MeshNodeTypeSource*+IndeterminateProbeReResolutionTest11/11.-c Release -warnaserror, 0 Warning(s) / 0 Error(s):MeshWeaver.Data,MeshWeaver.Graph,MeshWeaver.Documentation,MeshWeaver.Data.Test,MeshWeaver.Graph.Test.Not pinned by a test: that
Initializecalls the recorder at the right moments (theDeferaround the read, the three-arm.Do, the routing.Do) — drivingITypeSource.Initializeneeds internalsMeshWeaver.Datadoes not expose toGraph.Test. Review that part by reading.The verification #1122 still needs before it can be closed (unchanged in kind)
After a roll carrying this, the next occurrence's
DataContextline says which half held; paired with the sub-hubBuildupActionline (#5193/#5205) that is the whole answer. This PR does not say why a wait starves, and does not touch the FAILED-latch decision still open on the issue.Pairs-with: none — nothing public removed; one public interface and one public method added.
Implementers: none — no member added to an existing interface (
IReportsInitialLoadProgressis new and optional).Mirror-sync: none — no localization key added or changed.
🤖 Generated with Claude Code