console: fix stale catalog data across region switches - #38631
Conversation
A region switch re-hydrates the sync-engine collections under a new scope while the previous region's rows are still in memory. hydrate pointed the persist key at the new scope and scheduled a persist, so the old region's rows were written under the new region's cache key and served as a complete tree on the next load there. Treat a scope change as a fresh start: drop the pending persist and the in-memory rows, then seed from the new scope's own cache or flush the emptied set. Also clear a stale error when an empty pre-snapshot arrives: it is re-emitted when a reconnect attempt opens, and the UI should fall back to loading rather than hold the error through the backoff window.
On a region switch the connection manager updated its current address but only reconnected when the socket was disconnected. The socket was still connected to the previous region, so app-wide subscribes (cluster list, object explorer) kept streaming the old region's catalog until a full page refresh. Track the address of the most recent connection attempt and reconnect whenever the environment's address differs, including when the switch lands while a handshake is in flight.
While the re-subscribe at the new address is in flight, the atoms fed by useGlobalUpsertSubscribe still held the previous region's rows, so pages like the cluster list showed another region's catalog until the new snapshot arrived, or indefinitely if the new region is unhealthy. Reset the manager and atom to the loading state when the region changes, via a small useRegionChangeReset hook that the tests can exercise directly.
QA LLM Review1. HIGH -- a region round trip through a non-healthy region permanently kills the global subscribes
Details
Meanwhile Reachability is ordinary. Fix: pair the reset with a guaranteed teardown of the old connection, so the existing private pauseConnection() {
this.clearRetryTimer();
this.retryAttempt = 0;
+ this.connectInFlight = false;
+ this.target.disconnect();
this.notifyStateChange();
}Clearing 2. MEDIUM --
|
…vers Pausing on a non-healthy region left the socket connected to the previous region's address, so returning to a healthy region matched neither resume condition and the global subscribes never reconnected, while the region reset had already stopped the manager consuming rows. Disconnect on pause so the resume path reconnects, and clear connectInFlight since a pause landing mid-handshake gets no open or close callback for that socket.
useGlobalSubscribeCollection kept the previous region's completed snapshot in its manager, and the cross-region reconnect's onOpen replayed it into the new region's collection and scoped cache after hydrate had cleared them. Reset the manager on region change so the replay is an ignorable empty pre-snapshot.
|
Both findings addressed:
The |
The app-session subscribes accumulated one effect per concern across two hooks: manager creation, connection lifecycle, snapshot bridging, a collection keep-alive, and the region reset, with the ordering between them carrying the correctness burden. Consolidate each into a session object (createAtomSubscribeSession / createCollectionSubscribeSession) that wires the SubscribeManager, WebsocketConnectionManager, sink writes and region reset through direct store subscriptions, the same pattern WebsocketConnectionManager already uses for health. The hooks keep one effect that owns the session lifecycle, and no longer read the async environment atom, so they cannot suspend. The allObjects-to-collection bridge likewise moves to a store subscription, so atom ticks no longer re-render the mounting component. Sessions are plain objects, so their region behavior is tested headlessly and synchronously.
|
Follow-up commit The wiring now lives in plain session objects ( |
QA LLM Review1. MEDIUM -- the pause teardown kills every managed socket on a health-probe blip, including the SQL Shell's, and fires no close callback
DetailsA health verdict is not a socket verdict. Verified against this branch: with a real The Shell's Fix: scope the teardown to what it was added for, a socket pointing at a region the user has left, rather than to every unhealthy verdict. All nine private attemptedHttpAddress?: string;
+ private attemptedRegionId?: string;
@@ private handleEnvironmentChange
+ const regionId = this.store.get(currentRegionIdSyncAtom);
if (nowHealthy) {
...
} else {
- this.pauseConnection();
+ // Only tear down when the socket points at a region the user has left;
+ // a health blip in the current region leaves a working socket alone.
+ this.pauseConnection(regionId !== this.attemptedRegionId);
}
@@ private attemptConnection
this.attemptedHttpAddress = this.currentHttpAddress;
+ this.attemptedRegionId = this.store.get(currentRegionIdSyncAtom);
@@
- private pauseConnection() {
+ private pauseConnection(disconnect: boolean) {
this.clearRetryTimer();
this.retryAttempt = 0;
- this.connectInFlight = false;
- this.target.disconnect();
+ if (disconnect) {
+ // Clear connectInFlight too: this socket gets no open/close callback.
+ this.connectInFlight = false;
+ this.target.disconnect();
+ }
this.notifyStateChange();
}If you'd rather keep the unconditional teardown, the close notification is the part that has to change: an intentional |
3 participants
Motivation
Switching regions in the console left pages fed by the app-wide SUBSCRIBEs (cluster list, object explorer) showing the previous region's catalog until a full page refresh. Three related defects, in one PR because each is half a fix without the others:
The connection manager never reconnected on a region switch.
WebsocketConnectionManager.handleEnvironmentChangeupdatedcurrentHttpAddressbut only connected when the socket was disconnected — and it was still happily connected to the previous region. Fixed by tracking the address of the most recent connection attempt and reconnecting whenever the environment's address differs, including when the switch lands mid-handshake.The global subscribe atoms served the previous region's rows during (and, if the new region is unhealthy, after) the switch.
useGlobalUpsertSubscribenow resets the manager and atom to the loading state when the region changes, via a smalluseRegionChangeResethook.The sync-engine collection cache was poisoned across regions (flagged by the QA review on console: fix the sync-engine cache never seeding on load #38609):
hydrateon a scope change scheduled a persist of the rows still in memory, writing the old region's catalog under the new region's cache key, where the instant-load path would serve it as a complete tree. A scope change now drops the pending persist and in-memory rows, then seeds from the new scope's own cache or flushes the emptied set. The same commit clears a stale error when a reconnect's empty pre-snapshot arrives, so the UI falls back to loading instead of holding the error through the whole backoff window (the QA review's second finding).Tips for reviewer
One commit per defect, in reverse order of the list above. Regression tests accompany each: the scope-change and error-clear tests in
subscribeCollection.test.ts, the two region-switch reconnect tests inWebsocketConnectionManager.test.ts, anduseSubscribe.test.tsxfor the reset hook. Each was verified to fail against the code it fixes.Checklist
T-protolabel.🤖 Generated with Claude Code