SEPURI-SAI-KRISHNA opened a new pull request, #74181:
URL: https://github.com/apache/airflow/pull/74181
`task_state_store.set(key, value, retention=timedelta(hours=6))` is
documented as making the key expire after six hours. It does not. `expires_at`
is written on every path and read on none, so `get()` keeps returning the value
indefinitely.
Nothing else closes the gap. `airflow state-store clean` is the only thing
that acts on `expires_at`, and the cleanup page states that Airflow does not
purge those rows on a schedule and that cleanup must be triggered explicitly
via the CLI. On a deployment that has not cron-scheduled that command, no key
has ever expired.
**The change**
The two read paths, `_get_task_state_store` and `_aget_task_state_store`,
now skip rows whose `expires_at` has passed. `NULL` still means never expires,
which is what `set()` writes when no expiry is given.
Deliberately not filtered: `_delete_task_state_store` and
`_adelete_task_state_store`, because an expired key must still be deletable,
and the selects inside `cleanup()` and `_summary_dry_run()`, which exist to
find expired rows. `idx_task_state_store_expires_at` already indexes the column.
The two documentation pages contradicted each other, one describing expiry
and the other describing deletion eligibility, so both now say the same thing:
expiry is enforced on read, and cleanup reclaims the disk space afterwards.
**Tests**
Two tests, one per read path: a key past its expiry reads as absent,
synchronously and asynchronously. Both fail without the change and the other 45
tests in the file pass either way. I did not add a test for the `NULL` case,
because `set()` without an expiry already writes `NULL` and the existing
round-trip tests cover that branch, so removing the `IS NULL` arm fails them.
**Scope**
The admin REST API is left alone. `GET /dags/.../taskStateStore/{key}`
queries the model directly rather than through the backend and returns
`expires_at` in its response, which reads as an inspection surface, so it still
shows rows pending cleanup. Happy to make it 404 expired keys instead if you
would rather it match the SDK exactly.
Expired rows are not deleted on read. Keeping reads read-only avoids write
amplification on a hot path; `airflow state-store clean` and `airflow db clean`
remain the reclaim paths.
The asset state store is unaffected: `AssetStateStoreModel` has no
`expires_at` column, so there is nothing to enforce there.
Related but different: #68794 and #68793 were about `cleanup()` skipping
rows, and #66459 added the cleanup command itself. This is the read half, which
none of them covered.
A newsfragment will follow once the PR number exists.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes - Claude Code (Opus 5)
--
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]
