CS-11182 follow-up: widen cache invalidation to every reindex entry point#4900
Merged
Conversation
The original CS-11182 fix only fired the L2 `module_transpile_cache` bulk tombstone from `Realm.startReindex`'s post-completion `.then` — which covered POST `<realm>/_full-reindex` and `<realm>/_reindex` but nothing else. Production reindexes triggered via the operator-action endpoints (`/_grafana-reindex`, `/_grafana-full-reindex`, `/_post-deployment`), the publish-realm flow (`Realm.fullIndex`), and direct `enqueueReindexRealmJob` calls all bypassed `startReindex`. On staging today, a Grafana-button reindex of the base realm completed without ever tombstoning the L2 rows, so clients continued to be served pre-deploy bytes. Emit `notifyAllFileChanges(dbAdapter, realmURL)` from the worker side of `fromScratchIndex`, right after `IndexRunner.fromScratch` returns. The existing `realm_file_changes` wildcard listener on every replica then calls `realm.clearLocalSourceCaches()` — synchronous L1 wipe plus the fire-and-forget L2 bulk tombstone. One chokepoint covers every from-scratch trigger uniformly, including future ones. The regression test in `module-cache-race-test.ts` drives a reindex through `realm.realmIndexUpdater.fullIndex` (the bypass path that never wires up the original `startReindex` callback) and asserts the L2 rows are tombstoned. Verified to fail without the fix and pass with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Host Test Results 1 files ±0 1 suites ±0 1h 47m 51s ⏱️ - 1m 9s Results for commit 6d491f3. ± Comparison against earlier commit 20dd768. Realm Server Test Results 1 files ±0 1 suites ±0 8m 4s ⏱️ -25s Results for commit 6d491f3. ± Comparison against earlier commit 20dd768. |
Two CI-only failures on the previous commit: - `while (true)` in `waitForZeroLiveRows` tripped `no-constant-condition`. Rewrote the loop to test the condition explicitly so the lint rule is satisfied (and the code reads more directly). - The pre-existing failure-isolation test (`reindex still tombstones L2 rows when clearRealmDefinitions throws`) asserted `countLiveRowsForRealm() === 0` immediately after `await testRealm.reindex()` returned, but `#dropAllTranspiledModuleCacheEntries` fires the L2 bulk UPDATE as `void` (fire-and-forget). Locally the UPDATE landed before the next query; on slower CI runners it didn't, so the test was flaky. Both tests in that sub-module now go through the same poll-with-timeout helper, matching the new worker-side test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
module_transpile_cachebulk tombstone fromRealm.startReindex's post-completion.then, which coversPOST <realm>/_full-reindexand<realm>/_reindexbut nothing else./_grafana-reindex,/_grafana-full-reindex,/_post-deployment, the publish-realm flow (Realm.fullIndex), and directenqueueReindexRealmJobcalls all bypassstartReindex. On staging this morning a Grafana-button reindex of the base realm completed cleanly but never tombstoned the L2 rows — clients kept being served pre-deploy bytes (the originating symptom for CS-11182: "All Files" still not visible after the deploy + reindex).notifyAllFileChanges(dbAdapter, realmURL)afterIndexRunner.fromScratchreturns. Every replica's existingrealm_file_changeswildcard listener picks it up and callsrealm.clearLocalSourceCaches()(sync L1 wipe + fire-and-forget L2 bulk tombstone). One chokepoint covers every from-scratch trigger uniformly — current and future.The original
Realm.startReindex.thenis now strictly belt-and-suspenders for the POST/_full-reindexpath, which is fine to leave in place.Test plan
pnpm test-module module-cache-race-test.ts— new test asserts L2 rows are tombstoned after a reindex triggered viarealm.realmIndexUpdater.fullIndex(the bypass path that doesn't wire upstartReindex's.then).cards-grid.gtsflips from 13579 bytes (stale) to ~15573 bytes (new "All Files" code) andmodule_transpile_cache.body IS NULLfor the realm's rows.🤖 Generated with Claude Code