refactor: use queue-based concurrency coordinator - #623
Conversation
| } | ||
|
|
||
| try { | ||
| return coordinatorEvents.take(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for flagging this. I added singleWorkerForkJoinPoolDoesNotStarveCoordinator in ParallelIntegrationTest using a custom ForkJoinPool(1), two parallel branches, and a five-second timeout. It completes successfully, and the full ParallelIntegrationTest suite passes (65 tests).
On Corretto/OpenJDK 17, LinkedBlockingQueue.take() waits through an AQS ConditionObject; its condition node implements ForkJoinPool.ManagedBlocker, and ConditionObject.await() invokes ForkJoinPool.managedBlock. The pool therefore compensates for the blocked coordinator even though this call site does not invoke managedBlock explicitly. Wrapping take() in another managed blocker would be redundant for the supported runtime.
The regression coverage is in commit 12a99c2.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| try { | ||
| return coordinatorEvents.take(); |
There was a problem hiding this comment.
Codex AI review
[P1] Avoid unmanaged blocking on the configured ForkJoinPool. On Java 17, LinkedBlockingQueue.take() is not a ForkJoinPool.ManagedBlocker. With the newly tested ForkJoinPool(1), the handler's CompletableFuture.join() creates one compensating worker for this coordinator, which then parks here while the branch tasks remain queued, so the test times out and execution deadlocks. Use ForkJoinPool.managedBlock for this wait or restore a CompletableFuture-based event signal that allows pool compensation.
Codex AI reviewFound one high-severity coordinator liveness regression. Tests were not executed, as requested. Reviewed commit |
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Issue Link, if available
N/A
Description
Replace the concurrency operation's rebuilt
CompletableFuture.anyOfcoordination with a persistent event queue modeled after the Python SDK coordinator.Demo/Screenshots
N/A - internal coordination refactor with no user-facing UI changes.
Checklist
Testing
Unit Tests
Yes. Added coordinator queue admission and exceptional wakeup tests. Ran the full SDK unit suite: 1,139 tests passed.
Integration Tests
Existing map and parallel integration coverage was run: 169 tests passed, including replay, suspension, failures, nesting modes, early completion, and bounded concurrency.
Examples
Not applicable. This change does not alter the public API or add a customer-facing workflow.