sadpandajoe commented on code in PR #43782: URL: https://github.com/apache/superset/pull/43782#discussion_r3908674086
########## UPDATING.md: ########## @@ -84,9 +84,12 @@ completion transport for async chart-data — see the note on the interval poll) > When it is **enabled**, completion is delivered over the socket and the > recurring interval poll does not run; a one-shot `status_changes` catch-up on > waiter registration and on socket reconnect reconciles anything missed while -> disconnected. Redis Pub/Sub is lossy, so a `task.status` lost while the socket -> stays open is only recovered on the next reconnect/registration or, failing -> that, a per-request give-up timeout (a page reload re-establishes state). +> disconnected. The socket accelerates delivery over the authoritative +> `status_changes` API rather than replacing it: Redis Pub/Sub is best-effort +> (at-most-once, no replay), so a disconnect is reconciled by the catch-up on +> reconnect/registration; in the rare case a `task.status` is missed while the +> socket stays open, completion still resolves on the next reconnect/registration +> or the per-request give-up (and a page reload re-establishes state). Review Comment: A dropped `task.status` while the socket stays open does not resolve through the give-up path: `asyncEvent.ts` rejects the waiter with `Timed out waiting for chart-data query results`. Could this say that this case ends in a bounded error (or reload), rather than that completion still resolves? -- 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]
