Add synchronous join support to FORK_JOIN (#619)#680
Merged
nthmost-orkes merged 22 commits intoMar 12, 2026
Conversation
…oin-support-to-fork_join
25 tasks
v1r3n
reviewed
Mar 5, 2026
v1r3n
reviewed
Mar 5, 2026
nthmost-orkes
added a commit
to nthmost-orkes/conductor
that referenced
this pull request
Mar 6, 2026
Addresses reviewer feedback on PR conductor-oss#680 (v1r3n): 1. Change WorkflowTask.joinMode from String to the inner enum WorkflowTask.JoinMode { SYNC, ASYNC }, eliminating free-form string comparison and making the API self-documenting. 2. Read joinMode directly from taskModel.getWorkflowTask() in Join.getEvaluationOffset() instead of pulling it from the task's inputData map. The value is now never duplicated into the task payload. 3. Remove the joinMode copy from JoinTaskMapper.getMappedTasks() since Join no longer reads it from input data. 4. Update tests accordingly: - JoinTest: construct WorkflowTask with JoinMode enum; replace the now-redundant case-insensitivity test with a null-workflowTask test that verifies the async default. - JoinTaskMapperTest: use JoinMode enum; assert joinMode is absent from inputData and present on the workflowTask. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses reviewer feedback on PR #680 (v1r3n): 1. Change WorkflowTask.joinMode from String to the inner enum WorkflowTask.JoinMode { SYNC, ASYNC }, eliminating free-form string comparison and making the API self-documenting. 2. Read joinMode directly from taskModel.getWorkflowTask() in Join.getEvaluationOffset() instead of pulling it from the task's inputData map. The value is now never duplicated into the task payload. 3. Remove the joinMode copy from JoinTaskMapper.getMappedTasks() since Join no longer reads it from input data. 4. Update tests accordingly: - JoinTest: construct WorkflowTask with JoinMode enum; replace the now-redundant case-insensitivity test with a null-workflowTask test that verifies the async default. - JoinTaskMapperTest: use JoinMode enum; assert joinMode is absent from inputData and present on the workflowTask. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add ForkJoinSyncModeIntegrationTest with 9 test scenarios covering: all-succeed, backward compat (no joinMode), explicit ASYNC, optional branch fails, required branch fails, sequential tasks in branch, joinOn subset, nested fork/join, and 10-branch fan-out - Fix WorkflowTask.JoinMode proto field ID from 33→34 (conflict with `items` field added for DO_WHILE list iteration in a prior PR) - Add @ProtoEnum annotation to JoinMode enum so protogen can map it - Regenerate workflowtask.proto and AbstractProtoMapper with JoinMode - Add JSON workflow definition resources for manual/documentation use - Use raw JSON + RestTemplate for workflow registration to avoid classpath conflict with bundled conductor-client WorkflowTask Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace testcontainers/Redis setup with InMemoryQueueDAO inner @TestConfiguration to satisfy QueueDAO wiring without needing Docker - Evaluate JOIN tasks directly via @Autowired AsyncSystemTaskExecutor and Join bean (mirrors pattern used by Groovy Spock specs) - Set retryCount=0 on task defs so failed tasks fail immediately rather than being retried (required for failure-path tests 4 & 5) - Remove now-unnecessary testcontainers and DynamicPropertySource imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Reorder ObjectMapper import per spotless import ordering rules - Join pushIfNotExists signature onto one line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onTest - Join setUnackTimeout and forkTaskMap signatures to single lines - Split long fail() string concatenations per google-java-format rules Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ran ./gradlew spotlessApply to fix all remaining google-java-format violations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
Update on convergence: Replaced the
Convergence path for Orkes is now simple: subclass OSS |
The scheduled flush executor in PostgresPollDataDAO was never stopped, causing a resource leak and test flakiness. After CacheTest completes, its background thread continued writing cached poll data to the shared database every 200ms. When NoCacheTest subsequently ran its @before truncation, the rogue thread would re-insert a record immediately after, causing the post-truncation verification check to fail. Fixes: - Store ScheduledExecutorService reference and add @PreDestroy shutdown() - Add @DirtiesContext(AFTER_CLASS) to CacheTest so Spring closes its context (and calls @PreDestroy) before NoCacheTest begins Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 tasks
…utor-leak fix: stop PostgresPollDataDAO flush executor on context shutdown
v1r3n
approved these changes
Mar 12, 2026
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
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.
Implements #619 - adds property-based control for synchronous vs asynchronous join behavior in FORK_JOIN tasks.
Implementation
Property-Based Control
joinModeproperty toWorkflowTask(values:"SYNC"or"ASYNC")"ASYNC"for backward compatibilityJoinTaskMapperto join task input dataSynchronous Behavior via Evaluation Offset
Rather than overriding
isAsync()(which doesn't support per-task control), synchronous behavior is achieved through the evaluation offset mechanism:This approach works within the existing framework constraints while providing responsive synchronous-like behavior.
Usage Example
{ "name": "join_task", "taskReferenceName": "join_ref", "type": "JOIN", "joinOn": ["task1", "task2"], "joinMode": "SYNC" }Backward Compatibility
joinModecontinue using async behaviorTests
Convergence Notes: OSS vs Orkes Implementation
Current Orkes Implementation (OrkesJoin)
Orkes Conductor replaces the OSS
Joincomponent entirely withOrkesJoin:isAsync() = false- no property-based controlexpressionandevaluatorType(GraalJS)Convergence Path Options
Option 1: Orkes adopts OSS property-based approach
OrkesJoinwith enhanced OSSJoin+ property controljoinMode: "SYNC"by default in Orkes for backward compatibilityOption 2: Keep separate implementations
Implementation Compatibility
This PR's approach is convergence-friendly:
The property-based approach could serve as foundation for future convergence if Orkes wants to support both sync and async modes with user control.