fix(itc): await cross-worker propagation for role/schema mutations (#1497)#1499
Merged
Conversation
…1497) addRole, dropRole, createSchema, dropSchema, createAttribute and dropAttribute fired signalUserChange / signalSchemaChange without awaiting, returning success before sibling workers rebuilt their cached auth/schema state. A request routed to a lagging worker then observed stale state — spurious 403s on a fresh grant, brief continued access after a revoke, and describe_all reporting a just-created schema as missing. The ack machinery already exists: signal* returns broadcastWithAcknowledgement, which resolves only once every live worker has processed the event and acked, and each worker acks from its ITC handler only after rebuilding its cache. These call sites just dropped that promise. Awaiting it makes them consistent with the already-correct alterRole and add/alter/dropUser paths, enforcing the invariant that once a role/schema mutation returns success the change is in effect on every worker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Reviewed; no blockers found. |
kriszyp
requested review from
DavidCockerill
and removed request for
cb1kenobi and
heskew
…1497) broadcastWithAcknowledgement waited indefinitely for every live worker to ack. A dying worker is released by the port close listener, but a wedged-but-alive worker (event loop blocked, port still open) could hang a mutating admin/DDL op forever now that those ops await propagation. Add a bounded timeout (default 30s, overridable per call). On timeout we drive the still-pending per-port handlers through the same cleanup path an ack/close uses, log which worker threads didn't ack, and resolve best-effort — the durable write already succeeded and the health monitor restarts a truly stuck worker. A Set of pending handlers makes each port's handler idempotent across the ack, close, and timeout paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
signalUserChange / signalSchemaChange fired the local ITC handler (serverItcHandlers.user/.schema — which rebuilds THIS thread's auth/schema cache) without awaiting it, then returned the broadcast promise. So even after a mutating op awaited the signal, the originating worker returned success before its own cache finished rebuilding, and the next request it served observed stale state. Receivers were already covered (itc.js awaits the handler before acking); only the originator's local rebuild was fire-and-forget. This is the mechanism behind the THREADS_COUNT=1 northwind cross-schema-join flake: the worker that ran alter_role served the follow-up query off its own not-yet-rebuilt usersWithRolesMap. Await both legs (local handler + broadcast) so the op returns only once every thread, including the originator, is current. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ethan-Arrowood
approved these changes
Jun 29, 2026
Ethan-Arrowood
approved these changes
Jun 29, 2026
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
2 participants
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
Addresses the eventually-consistent ITC gap in #1497 across three layers:
Call-site awaits —
addRole,dropRole,createSchema,dropSchema,createAttribute, anddropAttributereturned success before sibling workers had rebuilt their cached auth/schema state, because they firedsignalUserChange/signalSchemaChangewithout awaiting the returned promise. Made consistent with siblings (alterRole,add/alter/dropUser) that already awaited.Originating-worker local rebuild —
signalUserChange/signalSchemaChangecalled the localserverItcHandlers.user/schema()fire-and-forget, then awaited only the cross-worker broadcast. So even if a caller awaited, the originating worker returned before its ownusersWithRolesMapwas rebuilt. Fixed withasync function + await Promise.all([localHandler, broadcast])inutility/signalling.ts.Ack timeout backstop —
broadcastWithAcknowledgementhad no timeout: a wedged-but-alive worker (hung ITC handler, not dead) would block admin ops forever. Added a best-effort 30 s timeout: remaining pending acks are resolved, thread IDs are logged, and the call returns so the durable write isn't rolled back.Root cause summary
The cross-worker ack machinery the issue proposes building already exists:
signalUserChange/signalSchemaChangereturnbroadcastWithAcknowledgement(server/threads/manageThreads.js), which resolves only after every live worker acks (a port close-listener releases the wait if a worker dies).server/threads/itc.js) only after awaitinguserHandler/schemaHandler— i.e. aftersetUsersWithRolesCache()/ schema reload completes.The bugs were: (a) six entry points dropped the returned promise; (b) the signal helpers themselves fired the local rebuild fire-and-forget.
Scope
createTable/dropTablepropagate through a different layer (harperBridge → ResourceBridge → Table apply path) — they don't use these signal helpers and are intentionally untouched.tokenAuthentication.tssignals on every login; awaiting there would add cross-worker latency for no auth-visibility benefit.Validation
Cannot reproduce the flake locally — the integration harness hardcodes
THREADS_COUNT=1, which prevents the multi-thread routing race. Using CI as the repro environment; watching whether the northwind cross-3-schema-join flake (thealter_rolestale-partial-403 from #1497 symptom 3) resolves after the local-rebuild fix.🤖 Generated with Claude Code — KrAIs