Repository navigation
feat: add invocation-owned executor isolation - #735
zhongkechen wants to merge 157 commits into
Conversation
5433d65 to
703c7e0
Compare
Pair request-local backend call/finally-exit IDs; require original peer task/wrapper occupancy at probe admission; establish a bounded child barrier before nested scheduling. Add counterexample and real-thread fixture tests, and align the fixture parent with the rebased 2.2.1 reactor. Locally reviewed and validated before publication.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
703c7e0 to
241c152
Compare
GitHub Actions
Inactive
![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.
GitHub Actions
Inactive
This comment has been minimized.
This comment has been minimized.
GitHub Actions
Inactive
| cleanupInterrupted.set(true); | ||
| executorService.shutdownNow(); | ||
| return combineFailures( | ||
| priorFailure, lifecycleFailure("Interrupted while shutting down invocation executor", interrupted)); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| shutdownFailure = normalizeOwnedExecutorFailure( | ||
| inspectOwnedExecutorFailure(shutdownFailure), isFatal(failure) ? (Error) failure : null); | ||
| failure = combineFailures(failure, shutdownFailure); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
Resolve the stacked-branch conflicts against fix/lmi-invocation-lifecycle at 318d78b while retaining invocation-owned executor cleanup, failure diagnostics, and the executor-isolation lifecycle tests. Validation: - Java 17 clean verify and Spotless: 3,985 passed, 33 conditional/cloud skips. - Java 25 focused lifecycle tests: 397 passed. - LMI Python fixture tests: 203 passed. - Packaged OpenTelemetry API compatibility: 4 cases passed.
GitHub Actions
Inactive
| shutdownFailure = normalizeOwnedExecutorFailure( | ||
| inspectOwnedExecutorFailure(shutdownFailure), isFatal(failure) ? (Error) failure : null); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_bzoq6c7q7wr72nt5c4p3wdy7xx
[P2] Preserve nonfatal wrapper diagnostics during cleanup
If the final isShutdown() query throws CompletionException(cause) with a suppressed diagnostic after successful shutdown, this call selects the nonfatal cause without transferring the wrapper's diagnostics. The wrapper is discarded before DurableExecutor can normalize it, so those failures become unreachable. Preserve diagnostics from every discarded completion wrapper while retaining application ExecutionException identity and cause graphs. Add a nonfatal state-query wrapper regression.
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.
| } catch (InterruptedException interrupted) { | ||
| cleanupInterrupted.set(true); | ||
| executorService.shutdownNow(); | ||
| return combineFailures( | ||
| priorFailure, lifecycleFailure("Interrupted while shutting down invocation executor", interrupted)); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_vgz5islhufc7okvs3qyjuzz6cb
[P2] Capture interruption before attempting forced shutdown
When awaitTermination() throws InterruptedException and shutdownNow() subsequently throws, this catch exits before creating the lifecycle failure containing the interruption. Although the interrupt flag is restored, the original exception is absent from the caller's diagnostic graph. Capture the interruption failure before invoking shutdownNow(), then aggregate any fallback failure while preserving fatal precedence. Extend the existing INTERRUPTED_FALLBACK regression to assert that both exceptions remain reachable.
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 reviewTwo P2 cleanup findings remain:
Static inspection only; tests were not executed. Reviewed commit |
Issue Link, if available
Fixes #840. Stacked on #733. General shared/nested bounded-pool starvation remains in #839; hard cleanup bounds and LMI worker recovery remain in #726.
Description
Concurrent LMI invocations can exhaust a shared bounded executor with root handlers waiting for queued steps.
withInvocationExecutorFactory(...)gives each invocation a fresh executor for its root, steps, child contexts and coordinators.withExecutorService(...)remains caller-owned and shared.The runtime uses the 3.x plugin factories and preserves main's handler-owned Start/End lifecycle. It waits for admitted work and End before shutting down an invocation-owned executor. Cancellation closes admission and requests interruption; non-cooperative work can still delay shutdown beyond the cleanup budget.
The factory must return a fresh, non-null, usable executor. An identity lease prevents concurrent reuse, and failed setup cleans up a newly acquired executor without touching one owned by another invocation. Configuring both executor modes is rejected.
toBuilder()and local replay preserve the resolved factories, while each invocation receives a new executor.Cleanup retains the selected lifecycle failure or first JVM fatal, with later setup, shutdown, termination and state-query failures attached as diagnostics. Discarded completion wrappers preserve their diagnostics without rereading cause accessors or creating suppression cycles. A retained application
ExecutionExceptionkeeps its identity and cause graph. Diagnostic transfer itself does not promote a suppressed fatal into the selected failure. Terminal ACK, delivery-error and implicit-close behavior retain their existing priorities.The LMI
fixed2fixture uses per-invocation fixed(2) pools. The sharednested2fixture remains diagnostic coverage outside the 12 gating cloud cases; this PR does not claim to solve arbitrary nested bounded-pool starvation.Validation
Validated tree:
205034e916871f71b9e27474661b18d5bfa5f30c, including main1c492229and the latest #733 implementation.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.