Don't let _mark_as_terminal clobber Perpetual successor state #335
Conversation
When a Perpetual task finishes, on_complete() calls docket.replace() to schedule the next run, which writes the successor's generation/state to the runs hash. Then _mark_as_terminal() unconditionally overwrites that with state=completed, or (with execution_ttl=0) deletes the entire hash. The successor becomes invisible. The fix uses a Lua script in _mark_as_terminal that atomically checks the generation counter before writing. If replace() already bumped the generation, the in-hand execution is superseded and the write is skipped. Progress cleanup and pub/sub notifications still fire unconditionally. For non-Perpetual tasks nothing changes — no replace() means the generation still matches and the write proceeds normally. Also parametrizes test_perpetual_race.py over execution_ttl to catch both the TTL=0 (hash deletion) and TTL>0 (state overwrite) variants, and adds a pub/sub event test for Perpetual completion. Relates to #331 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
📚 Documentation has been built for this PR! You can download the documentation directly here: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11f775586e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if current and tonumber(current) > generation then | ||
| return 'SUPERSEDED' |
There was a problem hiding this comment.
Returning SUPERSEDED here skips writing terminal state/result fields to runs:{key}, but Execution.get_result() still relies on a terminal pub/sub event followed by sync() against that hash; for Perpetual (or any running task replaced mid-flight), sync() now sees the successor's non-terminal state and get_result() falls through as if unfinished, returning None (and dropping failures/results) for an execution that actually completed. This is a behavior regression introduced by the supersession early-return path.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
=======================================
Coverage 98.67% 98.67%
=======================================
Files 103 103
Lines 10323 10375 +52
Branches 497 496 -1
=======================================
+ Hits 10186 10238 +52
Misses 121 121
Partials 16 16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
When execution_ttl=0, _mark_as_terminal DELs the runs hash after completion. If a stale message (from a replaced task) tries to claim() concurrently, the generation evidence is gone and the stale task runs. With real Redis the network latency means both messages usually claim before either completes, but the in-memory backend processes things instantly so one task can fully complete (and DEL the hash) before the other even starts. The fix: in the claim Lua script, if a generation-tracked message (gen > 0) finds no runs hash at all, it's stale — return SUPERSEDED. A generation-tracked message should always have a corresponding runs hash; if it's gone, a newer generation already completed and cleaned up. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
When a Perpetual task finishes,
on_complete()callsdocket.replace()toschedule the next run, which writes the successor's generation/state to
the runs hash. Then
_mark_as_terminal()unconditionally overwrites thatwith
state=completed, or (withexecution_ttl=0) deletes the entire hash.The successor becomes invisible.
The fix uses a Lua script in
_mark_as_terminalthat atomically checksthe generation counter before writing. If
replace()already bumped thegeneration, the in-hand execution is superseded and the write is skipped.
Progress cleanup and pub/sub notifications still fire unconditionally.
For non-Perpetual tasks nothing changes — no
replace()means thegeneration still matches and the write proceeds normally.
Also parametrizes
test_perpetual_race.pyoverexecution_ttlto catchboth the TTL=0 (hash deletion) and TTL>0 (state overwrite) variants,
and adds a pub/sub event test for Perpetual completion.
Relates to #331
🤖 Generated with Claude Code