You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(testing): surface manifest-fetch diagnostics on convergence timeout - #11438
Cosmos reminder scale-out intermittently exceeded the ten-second cluster-manifest convergence budget in #11434. The failing revision already included #11029, #11082, #11406, and #11116. The retained artifact recorded a GetSiloManifestHash request exceeding the thirty-second system RPC deadline, leaving the stalled observer's fetch and publication state to be determined.
Merged #11447 repairs a related transport publication race by completing both silo preambles before publishing the connection. Its deterministic regressions reproduce the ordering defect in both directions and its investigation addresses #11446 and #11434. The historical logs establish a common transport-failure signature; attribution of this specific recurrence remains subject to recurrence validation. This PR supplies durable observability for that validation and future recurrences.
Solution
Track internal per-attempt diagnostics in ClusterManifestProvider: membership version, start time, outstanding peer fetches, and the latest fetch failure. Each completion carries its originating attempt identity. Compare-and-swap updates preserve concurrent completions and carry forward failures recorded while the next attempt is installed. Cancellation is classified using the request token; independent peer cancellation is recorded and logged as a fetch failure.
Include each observer's manifest and attempt diagnostics in InProcTestCluster.WaitForTopologyToConvergeAsync timeout messages. Validate that the diagnostic reference remains stable across the manifest read, retrying when a completion or new attempt changes it. The returned values coexisted during the read. Their versions identify the published membership and the reconciliation attempt separately, including membership synchronized before the next attempt starts. Elapsed time uses the observer's own TimeProvider.
Cover the initial snapshot, successful completion, pending-set transitions, failures, stale completions, attempt-installation races, snapshot consistency, membership synchronization, cancellation classification, and exact timeout formatting with focused regressions.
Rationale
These internal diagnostics expose the stalled retrieval boundary while preserving topology convergence, provider validation, RPC deadlines, and retry behavior. They implement the issue's diagnostic outcome and complement the transport publication repair in #11447.
Improves diagnosability for #11434, which remains open for recurrence validation.
The current-main baseline is commit 48f3aaa47f and uses the same reviewed coverage matrix.
Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.
The comparison remains report-only while normal line and branch variance is calibrated.
Tester.Cosmos.Reminders.ReminderTests_Cosmos intermittently times out
with 'Cluster manifests did not converge within 00:00:10' (dotnet#11434).
CI-artifact analysis of a failing run traced this to a silo whose
GetSiloManifestHash RPC to a peer sat unprocessed for ~30s (the
default system RPC response timeout) before replying, even though
the handler is a synchronous, in-memory hash computation with no
I/O. That is consistent with scheduling/thread-pool contention in
the shared single-process InProcessTestCluster rather than a logic
defect in the manifest fetch/retry/peer-repair paths, all of which
already include the fixes from dotnet#11029, dotnet#11082, and dotnet#11406.
Per the issue's accepted resolution, add diagnostics-only
instrumentation (no behavioral change) so a recurrence is directly
diagnosable from the failure message instead of requiring manual
CI-artifact archaeology:
- ClusterManifestProvider now tracks a per-attempt snapshot
(membership version, attempt start time, silos still pending a
manifest fetch, and the most recent fetch failure) and updates it
as each individual fetch completes or fails, so pending silos
shrink in real time rather than only being know at attempt start.
- InProcTestCluster.WaitForTopologyToConvergeAsync resolves the
concrete internal ClusterManifestProvider and, on a convergence
timeout, appends per-observer pending-fetch diagnostics to the
thrown TimeoutException, taken from a single snapshot per observer
(manifest + diagnostics + the observer's own TimeProvider) so the
reported state is self-consistent even under a virtualized clock.
All new/changed members are internal; IClusterManifestProvider and
other public API are untouched, so no src/api regeneration is
required.
Adds focused tests for the new ManifestUpdateAttemptDiagnostics contract:
- LastAttemptDiagnostics is null before the first update attempt
- PendingSilos is fully cleared once all fetches in an attempt succeed
- PendingSilos shrinks per-fetch-completion in real time (not only after
the whole attempt completes), and failure fields (LastFailedSilo,
LastFailureMessage, LastFailureAt) are populated when a peer fetch fails
These exercise the new diagnostic output/state-transition contract itself,
complementing the pre-existing manifest-fetch success-path coverage.
- Bind MarkManifestFetchComplete to the attempt that started the fetch via a new AttemptId, preventing a stale direct-fetch completion from corrupting a newer attempt's diagnostics after peer repair.
- Add GetManifestAndDiagnosticsSnapshot() to pair LastAttemptDiagnostics with Current using a consistent read order, avoiding self-contradictory convergence-timeout messages.
- Make DescribePendingFetch internal and add direct formatter tests.
- Add a regression test proving a superseded attempt's completion is ignored.
- Format elapsed seconds in DescribePendingFetch using invariant culture so the diagnostic message (and its test) is not locale-dependent.
- Correct XML docs on GetManifestAndDiagnosticsSnapshot to accurately describe it as a best-effort pairing rather than an unconditional atomicity guarantee: a fetch completion or peer-repair publication between the two reads can still report a silo as both pending and already published.
Rebased on main to pick up #11447 (merged), which identifies and fixes the likely root cause of this flake: a premature-connection-publication race in SiloConnection/ConnectionManager (its own description states it "Addresses #11446 and #11434"). Updated this PR's description accordingly — it now adds durable observability rather than claiming to resolve root cause itself.
Also ran a rubber-duck self-review (gpt-6.1-sol) against the 3 code-review fixes and addressed both findings it raised:
DescribePendingFetch's elapsed-seconds formatting now uses invariant culture (was previously locale-dependent, which could also have made the new formatter test flaky on non-US-locale CI runners).
Corrected GetManifestAndDiagnosticsSnapshot's XML docs, which overstated the consistency guarantee: a fetch completion or peer-repair publication between the diagnostics read and the manifest read can still report a silo as both pending and already published. The method remains useful as a best-effort pairing for diagnostics; the docs now say so accurately instead of claiming an absolute guarantee.
This still does not produce the pairing promised by the method. Current can itself publish a newer partial manifest in EnsureValidManifestForCurrentMembership, so these exact two lines can read diagnostics for membership vN and then return a manifest at vN+1; a concurrent attempt can likewise install new diagnostics and publish before the second read. Reversing the reads only changes which inconsistent pairing is possible. The manifest and attempt diagnostics need a shared atomic/synchronized snapshot (or an equivalent consistency protocol) before this timeout output can reliably attribute pending fetches to the observed manifest.
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
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.
Problem
Cosmos reminder scale-out intermittently exceeded the ten-second cluster-manifest convergence budget in #11434. The failing revision already included #11029, #11082, #11406, and #11116. The retained artifact recorded a
GetSiloManifestHashrequest exceeding the thirty-second system RPC deadline, leaving the stalled observer's fetch and publication state to be determined.Merged #11447 repairs a related transport publication race by completing both silo preambles before publishing the connection. Its deterministic regressions reproduce the ordering defect in both directions and its investigation addresses #11446 and #11434. The historical logs establish a common transport-failure signature; attribution of this specific recurrence remains subject to recurrence validation. This PR supplies durable observability for that validation and future recurrences.
Solution
Track internal per-attempt diagnostics in
ClusterManifestProvider: membership version, start time, outstanding peer fetches, and the latest fetch failure. Each completion carries its originating attempt identity. Compare-and-swap updates preserve concurrent completions and carry forward failures recorded while the next attempt is installed. Cancellation is classified using the request token; independent peer cancellation is recorded and logged as a fetch failure.Include each observer's manifest and attempt diagnostics in
InProcTestCluster.WaitForTopologyToConvergeAsynctimeout messages. Validate that the diagnostic reference remains stable across the manifest read, retrying when a completion or new attempt changes it. The returned values coexisted during the read. Their versions identify the published membership and the reconciliation attempt separately, including membership synchronized before the next attempt starts. Elapsed time uses the observer's ownTimeProvider.Cover the initial snapshot, successful completion, pending-set transitions, failures, stale completions, attempt-installation races, snapshot consistency, membership synchronization, cancellation classification, and exact timeout formatting with focused regressions.
Rationale
These internal diagnostics expose the stalled retrieval boundary while preserving topology convergence, provider validation, RPC deadlines, and retry behavior. They implement the issue's diagnostic outcome and complement the transport publication repair in #11447.
Improves diagnosability for #11434, which remains open for recurrence validation.