Repository navigation
fix(bake): an unready replica neither re-keys shared NodeType records by adoption nor deadlocks its own activation sweep #5643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,19 +87,25 @@ public static TimeSpan CoordinationBackoff(int attemptsSoFar) => | |
| /// <param name="store">The shared assembly store, for the follower's post-GO probe.</param> | ||
| /// <param name="bake">The actual sweep to run when this process wins the claim.</param> | ||
| /// <param name="logger">Diagnostics.</param> | ||
| /// <param name="progress">Names, for the readiness message, what this process is waiting on | ||
| /// (#5544) — the claim election, or another replica's build.</param> | ||
| /// <returns>The sweep's outcomes (winner), or the post-GO probe's outcomes (follower).</returns> | ||
| public static IObservable<PreWarmOutcome> Run( | ||
| IMessageHub mesh, | ||
| NodeTypeBakeReport report, | ||
| IReadOnlyDictionary<string, NodeTypeDefinition?> definitions, | ||
| IAssemblyStore store, | ||
| Func<IObservable<PreWarmOutcome>> bake, | ||
| ILogger? logger) | ||
| ILogger? logger, | ||
| Action<string>? progress = null) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept as an optional parameter on purpose. The only consumer outside core is MeshWeaver.Plugins, and it compiles against a pinned core source (MW_PLATFORM_REF), not against a shipped binary, so there is no already-compiled caller to break. I tried the overload route first (in this branch) and it breaks SOURCE instead: every existing cref to Run / WarmDynamicTypes turns into CS0419 under -warnaserror (6 sites in core alone), and Plugins' DynamicTypePreWarmerTest.cs:108 has and :171 does typeof(DynamicTypePreWarmer).GetMethod(nameof(WarmDynamicTypes)), which throws AmbiguousMatchException once a second overload exists. Every current call site (BuildCoordinationTest.cs:128/256/304, DynamicTypePreWarmerTest.cs:71/235) uses positional or named arguments that still bind to the optional-parameter shape. |
||
| { | ||
| // Unique per process RUN: two boots of the same pod must not look like one claimant, or | ||
| // the second boot would inherit (and heartbeat) a claim whose bake died with the first. | ||
| var holder = $"{Environment.MachineName}/{Guid.NewGuid():N}"; | ||
| var fingerprint = report.FrameworkVersion; | ||
| progress?.Invoke( | ||
| $"asking for the build claim for framework {fingerprint} — the claim decides whether " | ||
| + "this process bakes or follows another replica's build"); | ||
|
|
||
| // A DEDICATED bake process outranks every serving pod in the claim election (#1424): when | ||
| // a bake Job is running, the pods lose deterministically, follow its GO, and never pay the | ||
|
|
@@ -137,7 +143,7 @@ public static IObservable<PreWarmOutcome> Run( | |
| .Catch((TimeoutException _) => Observable.Return(false)) | ||
| .SelectMany(granted => granted | ||
| ? BakeAsMaster(mesh, holder, fingerprint, definitions, bake, logger) | ||
| : FollowGo(mesh, holder, fingerprint, definitions, store, bake, logger))), | ||
| : FollowGo(mesh, holder, fingerprint, definitions, store, bake, logger, progress))), | ||
| mesh, fingerprint, definitions, store, logger); | ||
| } | ||
|
|
||
|
|
@@ -635,8 +641,14 @@ internal static IObservable<PreWarmOutcome> FollowGo( | |
| IReadOnlyDictionary<string, NodeTypeDefinition?> definitions, | ||
| IAssemblyStore store, | ||
| Func<IObservable<PreWarmOutcome>> bake, | ||
| ILogger? logger) | ||
| ILogger? logger, | ||
| Action<string>? progress = null) | ||
| { | ||
| // 🚨 #5544 — this wait is deliberately UNBOUNDED (see above), so it is the one wait whose | ||
| // readiness message must say what ends it: another replica's GO, or the claim falling free. | ||
| progress?.Invoke( | ||
| $"following another replica's build of framework {fingerprint} — waiting for its GO " | ||
| + "(or for its claim to fall free, which hands the bake to this process)"); | ||
| logger?.LogInformation( | ||
| "BuildProtocol: claim held elsewhere — {Holder} follows the build for framework " | ||
| + "{Fingerprint} (durable witness + this cluster's GO + its own claim candidacy)", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not changed, on purpose. It is the same lifecycle the two held COMPILE stamps have had since #3478, and it heals the same way. Ledger: NodeTypeAdoptionRegistry is only read by OverlayOnto when the sweep classifies, and that sweep has already run by the time a refusal and then a retraction can happen, so a stale ledger entry cannot misclassify anything later. Record: if the adoption was discarded and the process is re-admitted, the shared record still names the serving replicas' framework, so it reads as Foreign to this process (NodeTypeBuildIdentity.RefusalReason). The next activation of that type here takes DecideFrameworkStale -> Recompile (the process is not leaving, so there is no #5353 yield). That compile's stamp goes through the now-admitted gate and lands. Re-offering discarded publications on re-admission would give a refused image a replay path for everything it produced while refused. Discarding them is the property #3478 asks for.