Skip to content

[Fix #1329] Adding iterations to TaskContext#1337

Merged
fjtirado merged 2 commits intoserverlessworkflow:mainfrom
fjtirado:Fix_#1329
Apr 24, 2026
Merged

[Fix #1329] Adding iterations to TaskContext#1337
fjtirado merged 2 commits intoserverlessworkflow:mainfrom
fjtirado:Fix_#1329

Conversation

@fjtirado
Copy link
Copy Markdown
Collaborator

Fix #1329

Copilot AI review requested due to automatic review settings April 23, 2026 13:39
@fjtirado fjtirado force-pushed the Fix_#1329 branch 2 times, most recently from 4e91f5f to 59be63b Compare April 23, 2026 13:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an iteration counter to TaskContext to help distinguish repeated executions of the same task position (e.g., looped/switch tasks), and persists/restores that value through the persistence layer.

Changes:

  • Add iterations to TaskContext and populate it when creating a new task execution context.
  • Persist iterations for completed tasks via a new VERSION_2 encoding and propagate it through CompletedTaskInfo.
  • Restore persisted iterations when replaying completed tasks from persistence.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
impl/persistence/bigmap/src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BytesMapInstanceTransaction.java Bumps task-completed serialization to v2 and writes/reads iterations.
impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java Restores iterations from persisted CompletedTaskInfo into TaskContext.
impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/CompletedTaskInfo.java Extends persisted completed-task metadata with an iterations field (defaulting to 1 for older versions).
impl/core/src/main/java/io/serverlessworkflow/impl/executors/AbstractTaskExecutor.java Assigns iterations when constructing TaskContext and exposes an iterations() accessor.
impl/core/src/main/java/io/serverlessworkflow/impl/TaskContext.java Stores iterations on the task execution context and supports copy/restore via getter/setter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/TaskContext.java Outdated
@fjtirado fjtirado force-pushed the Fix_#1329 branch 2 times, most recently from bb168e7 to 677b1e9 Compare April 23, 2026 14:39
Copilot AI review requested due to automatic review settings April 23, 2026 14:39
@fjtirado fjtirado marked this pull request as draft April 23, 2026 14:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Signed-off-by: fjtirado <ftirados@redhat.com>
@fjtirado fjtirado force-pushed the Fix_#1329 branch 3 times, most recently from ba91735 to f0cbc8a Compare April 24, 2026 11:56
@fjtirado fjtirado marked this pull request as ready for review April 24, 2026 12:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowMutableInstance.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/TaskContext.java
Comment on lines +34 to +53
CompletableFuture<?> result = CompletableFuture.completedFuture(null);
for (Collection<WorkflowExecutionCompletableListener> listeners :
workflowContext.definition().application().listenersByPriority()) {
result =
result.thenCompose(
__ ->
CompletableFuture.allOf(
listeners.stream()
.map(
v ->
function
.apply(v)
.exceptionally(
ex -> {
logger.error("Error while executing listener", ex);
return null;
}))
.toArray(CompletableFuture[]::new)));
}
return result;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that listeners with different priority should not be executed concurrently (it kills the purpose of priority). listeners with higher priority should be completed before listeners with lower priority starts execution.
Since the list of listeners is not mutable once the application has been created, they are grouped by prioriry at application creation time, so there is not need to group them for every application change.

Comment on lines +64 to +66
if (info.tasks().isEmpty()) {
return;
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an optimization I forgot to add previously, once all persisted task are processed there is not point on performing a remove and two intances of

PersistenceTaskInfo taskInfo = info.tasks().remove(context.position().jsonPointer());
if (taskInfo == null) {
return;
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the same, there is not point in performing two instanceOf if the task is not in the map

Comment on lines +160 to +182
Collection<WorkflowExecutionCompletableListener> listeners) {
if (listeners.isEmpty()) {
return List.of();
}
List<Collection<WorkflowExecutionCompletableListener>> result = new ArrayList<>();
Iterator<WorkflowExecutionCompletableListener> iter = listeners.iterator();
List<WorkflowExecutionCompletableListener> currentList = new ArrayList<>();
WorkflowExecutionCompletableListener currentListener = iter.next();
int currentPriority = currentListener.priority();
currentList.add(currentListener);
while (iter.hasNext()) {
currentListener = iter.next();
if (currentListener.priority() != currentPriority) {
result.add(currentList);
currentList = new ArrayList<>();
currentPriority = currentListener.priority();
}
currentList.add(currentListener);
}
if (!currentList.isEmpty()) {
result.add(currentList);
}
return result;
Copy link
Copy Markdown
Collaborator Author

@fjtirado fjtirado Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is grouping listeners by priority.
Im using a list rather than map, since we do not really care about the priority number.
Since the source list is already sorted by priority, this old and simple algorithm will work.
It is done here because the list of listener is not mutable once the application is started to avoid repeating the grouping for every modification, which will negatively affect performance


int iteration();

short retryAttempt();
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a leftover from previous PR, retryAttemp make sense at interfaz level

@fjtirado fjtirado force-pushed the Fix_#1329 branch 2 times, most recently from e7e8da6 to 153612a Compare April 24, 2026 12:25
Copilot AI review requested due to automatic review settings April 24, 2026 12:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings April 24, 2026 12:44
Signed-off-by: fjtirado <ftirados@redhat.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fjtirado fjtirado merged commit 99a360d into serverlessworkflow:main Apr 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add task iteration on Task Context for repeteable tasks

3 participants