Skip to content

fix(#2684): modules GC leaves the readiness path — runs after ApplicationStarted through IIoPool - #2733

Merged
rbuergi merged 3 commits into
mainfrom
fix/2684-modules-gc-off-readiness-path
Aug 30, 2026
Merged

rbuergi merged 3 commits into
mainfrom
fix/2684-modules-gc-off-readiness-path

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #2684.

The defect

ModuleLandingService.CollectGarbage(moduleRoot) ran synchronously in the portal's boot path (MemexConfiguration.cs:207), before the host listened. On an Azure Files (CIFS) /data, the rename-then-recursive-delete of orphaned module generations is one SMB round-trip per file — minutes of uninterruptible IO for housekeeping that reclaims nothing the portal needs in order to serve. Rollout time thereby became a function of how much garbage the previous generation left on a network volume, unbounded and invisible until the probe kills the pod.

Live reproduction (right now): memex-cloud's stuck roll, helm revision 20 pending-upgrade. Pod memex-portal-deployment-69cc679d7d-88wds (image ci.6559) has PID 1 in state Dsl at wchan=wait_for_response, 3 s CPU in 15 minutes, open fds on /data/modules/.trash-MeshWeaver.Blazor.OpenStreetMap@7e17a082-…, zero stdout, no listener on :8080. The startup probe (60 × 5 s = 300 s) fired "Killing … failed startup probe" at 06:53:37, but the kill cannot land on a process parked in uninterruptible IO — so the roll loops, helm upgrade --wait timed out, and Deploy drift stays red. This PR does not touch the cluster; merging it produces the image whose next roll no longer has the defect.

The fix — the pass moves, its semantics do not

Root cause, not a probe-budget raise: generations GC comes off the readiness path entirely.

  • ModuleGenerationsGcHostedService (new, in MeshWeaver.PluginCatalog, registered by the same boot path that used to make the call — ahead of the awaiting-setup early return, so an instance parked in setup still reclaims): StartAsync only registers an IHostApplicationLifetime.ApplicationStarted callback, so it can never delay the listener, and nothing gates /health or /alive. The callback schedules the same CollectGarbage through the file-system IIoPool (InvokeBlocking) and subscribes with explicit error logging; the removal count is logged as today.
  • CollectGarbage observes cancellation between directories (the unit of atomic removal — the pool's token), so a mesh teardown cancels a sweep crawling a slow volume instead of waiting on it. Everything else is unchanged: delete only what no activation entry references and nothing holds open (skip-on-locked), landing never deletes, the atomic .trash-* rename (Module generations GC deletes lazily-loaded dependency DLLs out from under a running process; activation can point at a never-landed generation #2509), the Module activation entry survives without its landed assemblies — EntityViews views silently absent on a booted pod #2303 grace window, fail-closed on an unreadable reference set.
  • The reference set stays the effective one: the pass re-reads the per-module sidecar files at run time (fresher than what the boot-time pass saw, never staler), baseline Modules:Assemblies entries reference no generation directories by construction, and the running process is immune to its own reclaim because boot loads store-landed generations from the Module generations GC deletes lazily-loaded dependency DLLs out from under a running process; activation can point at a never-landed generation #2509 process-local pin — the same property that already protected it from a sibling pod's pass.
  • In-process reclaim stays — deferred, never dropped: no CronJob covers modules/. The chart's AssemblyCacheRetention sweep (and its ApplicationStarted + pool shape, which this service mirrors) covers only the NodeType assembly cache, not module generations.

Proof

  • ModuleGcOffReadinessPathTest (portal test project) drives the real ConfigureMemexMesh over a module root holding a backdated orphan generation and pins the three phases in order: configuring the mesh leaves it intact, StartAsync (everything before the listener) leaves it intact, and only ApplicationStarted triggers the pass that collects it — using the generic host's own ApplicationLifetime, no mocks.
  • ModuleGenerationsGcHostedServiceTest pins the seam: no sweep before ApplicationStarted (FIFO probe through the same cap-1 pool, not a sleep); a sweep stalled in IO — the memex-cloud Dsl shape, simulated as a collector parked until its token cancels — neither blocks the started callback nor survives StopAsync (disposal cancels the pooled leaf and the collector unwinds).
  • GenerationGcHardeningTest.CollectGarbage_StopsBetweenDirectories_WhenCancelled pins the cancellation contract: an already-cancelled pass removes nothing; a mid-pass cancel finishes the in-flight atomic removal and stops before the next directory; a later uncancelled pass still reclaims the survivor.
  • Existing ModuleLandingService suites stay green: 29/29 in MeshWeaver.PluginCatalog.Test (GenerationGcHardening + PendingLanding + ModuleGenerationsGcHostedService + ModuleLandingService), 34/34 in Memex.Portal.Shared.Test (ModuleGcOffReadinessPath + ConfigurationHandOver + EmailStartupConfigurationGuard).
  • MeshWeaver.PluginCatalog, Memex.Portal.Shared, both test projects and MeshWeaver.Documentation build Release -warnaserror, 0 warnings 0 errors.

Docs: Modules.md gains a "GC is OFF the readiness path" section; What's New entry (Category: Fix) included.

🤖 Generated with Claude Code

…r ApplicationStarted, through IIoPool

ModuleLandingService.CollectGarbage ran synchronously in ConfigureMemexMesh, before the
host listened. On an Azure Files (CIFS) /data the rename-then-recursive-delete of orphaned
module generations is one SMB round-trip per file — minutes of uninterruptible IO for work
that reclaims nothing the portal needs in order to serve. Rollout time became a function of
how much garbage the previous generation left on a network volume: memex-cloud's roll to
ci.6559 sat as PID 1 in Dsl at wchan=wait_for_response, never bound :8080, blew the 300 s
startup probe (whose kill cannot land on a process parked in uninterruptible IO) and looped,
wedging helm revision 20 in pending-upgrade.

The pass moves, its semantics do not:
- ModuleGenerationsGcHostedService (registered by the same boot path, ahead of the
  awaiting-setup early return) registers an ApplicationStarted callback in StartAsync —
  it can never delay the listener and gates neither /health nor /alive — and the callback
  schedules the SAME CollectGarbage on the file-system IIoPool (InvokeBlocking).
- CollectGarbage now observes a CancellationToken BETWEEN directories (the unit of atomic
  removal), so a mesh teardown cancels a sweep crawling a slow volume instead of waiting
  on it. Skip-on-locked, the #2303 grace window, the atomic .trash- rename (#2509) and the
  fail-closed unreadable-reference-set rule are unchanged; the reference set is re-read from
  the per-module sidecar files at run time, and the running process is immune to its own
  reclaim because it loads store-landed generations from the #2509 process-local pin.
- No CronJob covers modules/: the chart's AssemblyCacheRetention sweep covers only the
  NodeType assembly cache, so in-process reclaim stays — deferred, never dropped.

Proof: ModuleGcOffReadinessPathTest drives the real ConfigureMemexMesh over a module root
holding a backdated orphan and pins the three phases (configure: intact; StartAsync: intact;
ApplicationStarted: collected). ModuleGenerationsGcHostedServiceTest pins the seam (no sweep
before started; a stalled sweep neither blocks the started callback nor survives Stop), and
GenerationGcHardeningTest pins the between-directories cancellation contract.

Closes #2684.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 30, 2026 07:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves module-generations garbage collection off the portal readiness/startup path by deferring the existing ModuleLandingService.CollectGarbage(...) sweep to run after ApplicationStarted via the filesystem IIoPool, preventing slow network storage (e.g., CIFS/Azure Files) from blocking the listener and startup probe.

Changes:

  • Replace synchronous boot-time module GC with a new ModuleGenerationsGcHostedService that schedules GC after ApplicationStarted through IIoPool.
  • Add cancellation support to ModuleLandingService.CollectGarbage(...) so teardown can stop between directory removals.
  • Add targeted tests and documentation updates to pin the “off readiness path” contract and cancellation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/MeshWeaver.PluginCatalog.Test/ModuleGenerationsGcHostedServiceTest.cs Verifies GC runs only after ApplicationStarted and that teardown cancels a stalled sweep.
test/MeshWeaver.PluginCatalog.Test/GenerationGcHardeningTest.cs Adds a cancellation contract test ensuring GC stops between directories.
test/Memex.Portal.Shared.Test/ModuleGcOffReadinessPathTest.cs Drives the real Memex boot path and asserts GC is deferred until after the host starts listening.
src/MeshWeaver.PluginCatalog/ModuleLandingService.cs Adds cancellation support and documents the “housekeeping, not readiness” rule.
src/MeshWeaver.PluginCatalog/ModuleGenerationsGcHostedService.cs Introduces the hosted service + DI registration extension to run GC post-start via IIoPool.
memex/Memex.Portal.Shared/MemexConfiguration.cs Replaces boot-time GC call with hosted-service registration.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-module-cleanup-no-longer-delays-a-restart.md Adds a What’s New “Fix” entry describing the operational improvement.
src/MeshWeaver.Documentation/Data/Architecture/Modules.md Documents GC timing changes and rationale (“off readiness path”).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.PluginCatalog/ModuleLandingService.cs
Comment thread src/MeshWeaver.PluginCatalog/ModuleLandingService.cs
…view)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

701 tests   509 ✅  4m 54s ⏱️
  5 suites  192 💤
  5 files      0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

916 tests   916 ✅  5m 28s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 533 tests   1 533 ✅  6m 22s ⏱️
    8 suites      0 💤
    8 files        0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

   10 files     10 suites   5m 56s ⏱️
3 466 tests 3 466 ✅ 0 💤 0 ❌
4 110 runs  4 110 ✅ 0 💤 0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

968 tests   968 ✅  5m 55s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

793 tests   789 ✅  6m 47s ⏱️
  6 suites    4 💤
  6 files      0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   41 files     41 suites   35m 23s ⏱️
8 377 tests 8 181 ✅ 196 💤 0 ❌
9 021 runs  8 825 ✅ 196 💤 0 ❌

Results for commit 4d7812d.

♻️ This comment has been updated with latest results.

… not a kernel wait

BlockingBridgeInTestRatchetGuard (#2013) refuses a new .Wait() site in test code, and rightly:
a thread parked in a native wait is outside methodTimeout's reach. The stalled-sweep test
now spins cooperatively until the pooled leaf's token cancels, with a 30 s ceiling — the
property it proves (the started callback and Stop never wait on a stalled sweep) is
unchanged, and there is no wait a broken cancellation path could park forever.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rbuergi
rbuergi merged commit 28a8000 into main Aug 30, 2026
36 of 38 checks passed
@rbuergi
rbuergi deleted the fix/2684-modules-gc-off-readiness-path branch August 30, 2026 08:40
rbuergi added a commit that referenced this pull request Aug 30, 2026
…ree racing terminals won

HandleGetDataRequest's arms sit behind one CAS precisely because they race — this class's own
remarks say so — and the frozen-creation fault can legitimately win, answering
GetDataResponse{Error='Exception has been thrown by the target of an invocation'}. That is still a
terminal refusal: the caller can tell 'will never answer' from 'still working', which is the entire
point of the NACK. Asserting only the DeliveryFailure arm made the test lose a race it was never
testing — it fired today on #2724, #2721, #2733 and #2743, none of which can reach this code, and
each cost an investigation.

The owner-side message assertions (AnsweredByTheOwner, the 'shutting down' marker, the absence of
'No node found') move INTO the DeliveryFailure arm, where they describe the thing they check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Boot-time modules GC over Azure Files gates readiness — a roll blows the 300s startup probe and loops

2 participants