Repository navigation
fix(cron): external-worker one-shot runs no longer book failed on a half-written ack (#116164, salvage #107678) - #116689
Merged
Merged
Conversation
_run_external_worker_payload wrote the acknowledgement JSON directly into ack_path after creating it with O_CREAT|O_EXCL. That made an empty 0-byte file visible to the scheduler's exists()-then-read polling loop (every 50ms) before the JSON body was flushed, so the poller would occasionally read an empty file and crash with json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0) (observed 3x on 2026-09-09 in production logs). Fix: write the ack payload to a per-pid temp file in the same directory, then os.replace() it into place. Readers now only ever see ack_path fully absent or fully written.
…envc's author mapping The worker ack file is what the gateway's exists()-then-read loop keys on; the test drives the production entry point _run_external_worker_payload and asserts the ack is never observable while its body is still being written (red on origin/main: O_CREAT made the empty file visible before json.dump), and that no temp residue is left beside it. Live probe (real `python -m cron.scheduler --external-worker-file` on a scratch HERMES_HOME, tight parent-style poller): origin/main 3/3 runs read an empty ack -> "unreadable acknowledgement"; this head 5/5 read a full ack. Refs #116164 (form 1), #107184. Salvages #107678 (@keenvc).
૮ >ﻌ< ა ci reviewran on b577683 — test(cron): one invariant for the atomic external-worker ack debug infoCI timingsCI timings · View report · View jobWall time 6m12s vs 6m29s (-4.4%). 1 job(s) slower, 9 faster, 2 unchanged.
|
alt-glitch
added
type/bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One-shot cron runs dispatched to a restart-safe external worker are no longer booked
failedwith "Fire claim ownership lost before terminal completion." when the worker's ready-acknowledgement is read while half-written: the ack file is now published atomically.cron/scheduler.py::_run_external_worker_payloadwrites the ack JSON to a per-pid.tmpsibling, fsyncs it andos.replace()s it into place (contributor commit, @keenvc). The gateway'sexists()-then-read poll (_spawn_external_cron_worker, 50 ms) can only ever observe the ack fully absent or fully written; the "published an unreadable acknowledgement; treating handoff as ownership-uncertain" branch that terminalized a successful, delivered run asfailedis no longer reachable from a normal handoff._run_external_worker_payloadand asserts the ack is not observable while its body is being produced and that no temp residue remains (red onorigin/main:O_CREATmade the empty file visible beforejson.dump).contributors/emails/mapping for the contributor's commit email.Live repro (real
python -m cron.scheduler --external-worker-fileon a scratchHERMES_HOME, parent-style tight poller)origin/mainJSONDecodeError→ "unreadable acknowledgement"execution_idmatchesRoot cause: the worker created the ack with
O_CREAT|O_EXCLand then wrote the body into the same path, so the file existed empty for the window betweenopen()andjson.dump(), which the parent's exists-then-read poll hit.The reporter's "form 2" ("Timed out waiting for local fire fence … failing closed" while delivery holds the fence past the 30 s lock wait) is a different mechanism and is handled with #116136.
Fixes #116164
Salvages #107678 (@keenvc) — commit cherry-picked with authorship preserved.
Dropped hunks
None — the contributor's single commit is applied in full; the test is ours.
Infographic