villebro opened a new pull request, #43734:
URL: https://github.com/apache/superset/pull/43734
### SUMMARY
Addresses the latest review of the GTF async chart-data work (findings
P1a/P1b/P1c and the DAG-scheduling concern P2a), and adds transition/deferral
metrics. Targets the `gaq-to-gtf` umbrella branch.
**P1c — non-abortable timeout no longer strands a task `IN_PROGRESS`.** A
task
with no registered abort handlers can't be cancelled, so its timeout firing
is
now a logged no-op. Previously it set `_timeout_triggered`, which blocks the
executor's `IN_PROGRESS → SUCCESS` transition (gated on no abort in flight),
while the finalize only moves `TIMED_OUT` from `ABORTING` — a transition that
never happens for a handler-less task — leaving it stuck `IN_PROGRESS`. It
now
runs to natural completion.
**P2a — the DAG dependency gate is non-blocking.** A dependent picked up
before
its prerequisites are terminal is deferred with a Celery
`self.retry(countdown=…,
max_retries=None)` instead of parking a worker slot in a blocking
`wait_for_completion`. The backoff grows and is jittered (1s, 3s, 5s… capped
at
30s, keyed off `self.request.retries`, so no persisted state). The gate
(`_unmet_prerequisite`) trusts a prerequisite already terminal in the loaded
snapshot and re-reads only the non-terminal ones; the abort-before-claim
check
and the failed-prerequisite `FAILURE` cascade moved into `execute_task`,
ahead of
the heartbeat/claim, so a merely-deferred task carries no heartbeat and
can't be
mistaken for abandoned work by the reaper.
**P1b — a failed Celery enqueue no longer poisons the dedup key.** If
`execute_task.delay(...)` throws, the just-created task is transitioned to
`FAILURE` (and a completion published) instead of being left `PENDING`,
where it
would dedup future identical submits into a task that never runs. The
original
enqueue error is re-raised; cleanup failures are swallowed and logged (the
orphan
reaper is the backstop).
**Observability.** Every status transition emits
`gtf.task.transition.<status>`,
and each DAG defer emits `gtf.task.dag_deferred`.
**P1a — forced async chart-data no longer double-executes.** A forced
refresh in
the async flow is issued twice — the async submit, then a synchronous
read-back —
and both previously carried `force=true`, so the read-back recomputed the
identical query instead of reading the result the task just cached. A
per-refresh
idempotency nonce (`force_nonce`) now dedups this **server-side**:
- The first execution to run for a `(nonce, cache_key)` recomputes with
`force`
and, *after* its result is cached, records a marker
`gtf-force-nonce:{nonce}:{cache_key}`.
- Any later request carrying the same nonce sees the marker and resolves
`force=false`, reading the freshly-warmed result.
- The marker is a **separate** cache key, never folded into the result
`cache_key`, so the fresh value lands under the normal key and ordinary
(non-forced) loads stay warm.
- A request whose `cache_key` differs from the one the forced compute cached
against (e.g. templated `{{ current_timestamp }}` / time-relative keys that
shift between submit and read-back) finds no marker and correctly
recomputes,
rather than mis-reading another key's result. The composite key also makes
each
query object in a multi-query request independently idempotent under one
request nonce.
The nonce is minted once per user force refresh on the frontend
(`requestChartDataResolved`) and carried on both requests; it is threaded
through
query serialization so the async task and the read-back share it. Because the
guard is server-side, it also covers retries, double-clicks, and second-tab
refreshes — not just the re-issue.
Because the design is nonce-based (not "clear `force` on the re-issue"), the
frontend keeps `force=true` on both requests; the server, not the client, is
what prevents the second compute.
### TESTING INSTRUCTIONS
- Backend: `pytest tests/unit_tests/tasks/ tests/unit_tests/commands/tasks/
tests/unit_tests/common/`
(DAG gate/defer, enqueue-failure cleanup, timeout no-op, transition metric,
nonce gate + marker, serialization round-trip).
- Frontend: `npm run test -- chartActions buildQueryContext exploreUtils`
(one nonce minted per refresh and carried on both submit and re-issue; both
keep `force`; the re-issue is synchronous).
- Manual (async chart-data deployment, `GLOBAL_ASYNC_QUERIES` on):
force-refresh
a chart; confirm the query runs **once** (one task, no second compute on
the
read-back) via query logs/metrics; a fresh force refresh recomputes.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API (adds optional `force_nonce` to the
chart-data query context; ignored when `force` is false)
- [ ] Removes existing feature or API
--
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]
