Repository navigation
fix: enforce cooperative invocation deadlines - #783
zhongkechen wants to merge 7 commits into
Conversation
Use an invocation-local monotonic budget for SDK admission and waits. Reject newly registered descendants of completed contexts before user code or checkpoint dispatch. Preserve acknowledged terminal outcomes without issuing metadata reads after the cutoff. Keep running threads confined and document the unresolved non-cooperative cleanup boundary in #741.
GitHub Actions
Error
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
Preserve deadline checks alongside checkpoint-token revocation handling and retain both invocation deadline and invoke propagation documentation. Refresh new invocation-context test fixtures and cover cleanup-time expiry with revocation and accepted terminal checkpoints without a token.
GitHub Actions
Error
Allow saved checkpoint reads and scoped ReplayChildren reconstruction without admitting new orphaned work, including virtual child contexts. Settle terminal checkpoint requests across the invocation cutoff before classifying their outcomes. Cover same-invocation map/parallel resumes, saved successes and errors, reconstruction scope cleanup, late terminal transport outcomes, and expired terminal requests that were never dispatched.
| replay = allow_replay and ( | ||
| self.get_checkpoint_result(identifier.operation_id).is_terminal() | ||
| or (is_virtual and _RECONSTRUCTING_COMPLETED_CONTEXT.get() is self) | ||
| ) |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ygsoiugckiy5x5urrhaxumbgjl
[P1] Allow saved callback IDs during completed-child reconstruction
An existing create_callback() only reads its checkpointed callback ID, including while its status is STARTED. If a large-result child creates a callback and completes with ReplayChildren, then its map/parallel branch suspends and resumes within the same invocation, this terminal-only exception rejects that saved callback as orphaned. The coordinator handles ORPHANED by exiting and can checkpoint a successful batch with the resumed branch's result missing. Extend replay admission to existing callback checkpoints while retaining guards against new callback creation, and add this callback case to the same-invocation replay tests.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| output = plugin_executor._run_in_invocation_context( | ||
| invoke_handler | ||
| ) | ||
| execution_state.check_invocation_result() |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_lee6rhbf3w2hdig4iho3ezcsim
[P2] Preserve permanent checkpoint failures after deadline settlement
If a terminal checkpoint remains in flight past the cutoff and then raises a nonretryable CheckpointError, the settlement wait delivers that error and invoke_handler() correctly produces FAILED. This check then sees no terminal acknowledgment and replaces the classified failure with InvocationDeadlineExceeded. A permanent API rejection therefore triggers another Lambda retry and loses its error in the returned outcome. Preserve nonretryable API failures through both final deadline checks, and extend the terminal-transport test with DurableApiErrorCategory.EXECUTION; its current OSError case only covers retryable failures.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| # Cleanup hooks and all owned workers have returned. A hook can consume | ||
| # the remaining budget; do not call End again when this check fails. | ||
| execution_state.check_invocation_result() |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ecw76eou64pajwr5zyatsluwvv
[P2] Keep invocation-end status consistent with the wrapper outcome
For a small successful result, no terminal checkpoint is sent. If an on_invocation_end hook consumes the remaining budget, this check raises InvocationDeadlineExceeded after every End hook has already received SUCCEEDED. Both bundled OTel plugins export the terminal workflow span before calling force_flush(), so a slow flush causes Lambda to retry an execution already reported as finished. Finalize the outcome before reporting terminal status to plugins—for example, acknowledge the terminal result before End so it survives subsequent cleanup—and add a slow-End integration test asserting both wrapper and telemetry outcomes.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewThree actionable findings:
Static inspection only; tests were not executed. Reviewed commit |
This branch was successfully deployed
1 participant
Expired Lambda invocations could start new user work, and SDK-owned waits could keep cooperative branches pinned during invocation cleanup. This change captures one monotonic invocation budget, rejects late work, and lets those waits unwind as a retryable invocation interruption while preserving checkpointed outcomes.
Related to #741. This is a partial implementation and intentionally does not close that issue. Regression test PR #743 remains separate and unchanged.
Remaining boundary: this does not provide a hard bound on the wrapper or guarantee LMI worker reclamation. Arbitrary user code/I/O, an already-running synchronous service call (including retries), and blocked plugin hooks still have to return before their threads can be joined. No threads are detached or forcibly interrupted. Non-cooperative worker retirement needs a runtime-supported strategy and real LMI validation; #741 remains open for that work. Shared conformance requirements for that complete contract should be coordinated separately.
Current head
6aed2267also includes upstream main04daf408. The incoming change touches only the standalone MicroVM worker and its tests; LMI/core/OTel source trees remain unchanged. That worker tree is identical in both refreshed Python PRs and passed 261 tests, type checking (13 files), and formatting/lint. Current-head CI is tracked separately.Validation on the reviewed implementation:
git diff --checkpass.