villebro commented on code in PR #43696:
URL: https://github.com/apache/superset/pull/43696#discussion_r3891455293
##########
superset-frontend/src/middleware/asyncEvent.ts:
##########
@@ -275,8 +297,10 @@ const loadStatusChanges = async (generation: number) => {
// Start (or wake) the poll loop for a freshly registered waiter: poll eagerly
// again, and kick the loop if it had gone idle. Idempotent — a no-op while the
-// loop is already running or when async queries are disabled.
+// loop is already running, when async queries are disabled, or when the
websocket
+// is the transport (WS mode never polls; reconnect catch-up reconciles
instead).
const ensurePolling = () => {
+ if (wsEnabled) return;
Review Comment:
This is a deliberate design decision rather than an oversight. The point of
the websocket path is to stop the parallel polling that was loading the
backend, so we specifically don't want a recurring status check running while
the socket is healthy.
The lossy-while-open window is narrower than it first looks: WebSocket
delivery over an established TCP connection isn't silently dropped — loss
implies the connection breaks, which fires `onclose` → reconnect → the one-shot
catch-up. The residual case is a pod's Redis→Node subscriber blip while the
browser socket stays open, which is pod-level and uncommon. We reconcile on
both waiter registration and reconnect (the common cases), and a per-request
give-up bounds a genuinely-lost message; a page reload re-establishes state.
If dropped `task.status` over Pub/Sub turns out to be a real problem in
practice, the right fix is to revisit Pub/Sub vs Redis Streams for this channel
(persisted, at-least-once delivery) rather than reintroducing a steady-state
polling backstop — we'd prefer not to pay the continuous polling cost for a
rare failure mode. I'll add a follow-up issue to track the Streams option so
it's on record.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
