Skip to content

Commit f7c7341

Browse files
Quinn-With-Two-NsQuinn-With-Two-Ns
authored
Fix transition in LA when handling canceled child wf (#2156)
* Fix transition in LA when handling canceled child wf
1 parent b92c97d commit f7c7341

File tree

6 files changed

+626
-4
lines changed

6 files changed

+626
-4
lines changed

‎temporal-sdk/src/main/java/io/temporal/internal/statemachines/LocalActivityStateMachine.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ enum State {
121121
State.REQUEST_PREPARED,
122122
LocalActivityStateMachine::sendRequest)
123123
.add(State.REQUEST_PREPARED, ExplicitEvent.MARK_AS_SENT, State.REQUEST_SENT)
124+
// This is to cover an edge case where the event loop is
125+
// run more than once while processing a workflow task.
126+
// This can happen due to external cancellation
127+
.add(
128+
State.REQUEST_PREPARED,
129+
ExplicitEvent.NON_REPLAY_WORKFLOW_TASK_STARTED,
130+
State.REQUEST_PREPARED)
124131
.add(
125132
State.REQUEST_SENT,
126133
ExplicitEvent.NON_REPLAY_WORKFLOW_TASK_STARTED,

‎temporal-sdk/src/main/java/io/temporal/internal/statemachines/LocalActivityStateMachine.puml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ EXECUTING --> REQUEST_PREPARED: SCHEDULE
3131
MARKER_COMMAND_CREATED --> RESULT_NOTIFIED: RECORD_MARKER
3232
REPLAYING --> WAITING_MARKER_EVENT: SCHEDULE
3333
REQUEST_PREPARED --> REQUEST_SENT: MARK_AS_SENT
34+
REQUEST_PREPARED --> REQUEST_PREPARED: NON_REPLAY_WORKFLOW_TASK_STARTED
3435
REQUEST_SENT --> REQUEST_SENT: NON_REPLAY_WORKFLOW_TASK_STARTED
3536
REQUEST_SENT --> MARKER_COMMAND_CREATED: HANDLE_RESULT
3637
RESULT_NOTIFIED --> MARKER_COMMAND_RECORDED: MARKER_RECORDED

‎temporal-sdk/src/test/java/io/temporal/internal/statemachines/LocalActivityStateMachineTest.java

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,25 @@
2727
import static org.junit.Assert.fail;
2828

2929
import io.temporal.api.command.v1.Command;
30+
import io.temporal.api.command.v1.StartChildWorkflowExecutionCommandAttributes;
3031
import io.temporal.api.common.v1.ActivityType;
3132
import io.temporal.api.common.v1.Payloads;
33+
import io.temporal.api.common.v1.WorkflowExecution;
3234
import io.temporal.api.enums.v1.CommandType;
3335
import io.temporal.api.enums.v1.EventType;
34-
import io.temporal.api.history.v1.MarkerRecordedEventAttributes;
36+
import io.temporal.api.history.v1.*;
3537
import io.temporal.api.workflowservice.v1.PollActivityTaskQueueResponse;
3638
import io.temporal.api.workflowservice.v1.RespondActivityTaskCompletedRequest;
3739
import io.temporal.common.converter.DataConverter;
3840
import io.temporal.common.converter.DefaultDataConverter;
3941
import io.temporal.internal.history.LocalActivityMarkerUtils;
4042
import io.temporal.internal.worker.LocalActivityResult;
43+
import io.temporal.workflow.ChildWorkflowCancellationType;
44+
import io.temporal.workflow.Functions;
4145
import java.util.ArrayList;
4246
import java.util.List;
4347
import java.util.Optional;
48+
import java.util.concurrent.atomic.AtomicReference;
4449
import org.junit.AfterClass;
4550
import org.junit.Test;
4651

@@ -343,4 +348,99 @@ protected void buildWorkflow(AsyncWorkflowBuilder<Void> builder) {
343348
List<Command> commands = stateMachines.takeCommands();
344349
assertTrue(commands.isEmpty());
345350
}
351+
352+
@Test
353+
public void testLocalActivityStateMachineDuplicateTask() {
354+
class TestListener extends TestEntityManagerListenerBase {
355+
@Override
356+
protected void buildWorkflow(AsyncWorkflowBuilder<Void> builder) {
357+
StartChildWorkflowExecutionParameters childRequest =
358+
new StartChildWorkflowExecutionParameters(
359+
StartChildWorkflowExecutionCommandAttributes.newBuilder(),
360+
ChildWorkflowCancellationType.WAIT_CANCELLATION_REQUESTED);
361+
ExecuteLocalActivityParameters parameters1 =
362+
new ExecuteLocalActivityParameters(
363+
PollActivityTaskQueueResponse.newBuilder()
364+
.setActivityId("id1")
365+
.setActivityType(ActivityType.newBuilder().setName("activity1")),
366+
null,
367+
System.currentTimeMillis(),
368+
null,
369+
false,
370+
null);
371+
// TODO: This is a workaround for the lack of support for child workflow in the test
372+
// framework.
373+
// The test framework has no support for state machines with multiple callbacks.
374+
AtomicReference<Functions.Proc> cc = new AtomicReference<>();
375+
AtomicReference<Functions.Proc2<Optional<Payloads>, Exception>> completionCallback =
376+
new AtomicReference<>();
377+
builder
378+
.<WorkflowExecution, Exception>add2(
379+
(r, c) ->
380+
cc.set(
381+
stateMachines.startChildWorkflow(
382+
childRequest,
383+
c,
384+
(r1, c1) -> {
385+
completionCallback.get().apply(r1, c1);
386+
})))
387+
.add((r) -> cc.get().apply())
388+
.<Optional<Payloads>, Exception>add2(
389+
(r, c) -> {
390+
completionCallback.set(c);
391+
})
392+
.<Optional<Payloads>, LocalActivityCallback.LocalActivityFailedException>add2(
393+
(r, c) -> stateMachines.scheduleLocalActivityTask(parameters1, c));
394+
}
395+
}
396+
/*
397+
1: EVENT_TYPE_WORKFLOW_EXECUTION_STARTED
398+
2: EVENT_TYPE_WORKFLOW_TASK_SCHEDULED
399+
3: EVENT_TYPE_WORKFLOW_TASK_STARTED
400+
4: EVENT_TYPE_WORKFLOW_TASK_COMPLETED
401+
5: EVENT_TYPE_START_CHILD_WORKFLOW_EXECUTION_INITIATED
402+
6: EVENT_TYPE_CHILD_WORKFLOW_EXECUTION_STARTED
403+
7: EVENT_TYPE_WORKFLOW_TASK_SCHEDULED
404+
8: EVENT_TYPE_WORKFLOW_TASK_STARTED
405+
9: EVENT_TYPE_WORKFLOW_TASK_COMPLETED
406+
10: EVENT_TYPE_REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_INITIATED
407+
11: EVENT_TYPE_EXTERNAL_WORKFLOW_EXECUTION_CANCEL_REQUESTED
408+
12: EVENT_TYPE_WORKFLOW_TASK_SCHEDULED
409+
13: EVENT_TYPE_WORKFLOW_TASK_STARTED
410+
*/
411+
TestHistoryBuilder h =
412+
new TestHistoryBuilder()
413+
.add(EventType.EVENT_TYPE_WORKFLOW_EXECUTION_STARTED)
414+
.addWorkflowTask()
415+
.add(
416+
EventType.EVENT_TYPE_START_CHILD_WORKFLOW_EXECUTION_INITIATED,
417+
StartChildWorkflowExecutionInitiatedEventAttributes.newBuilder().build())
418+
.add(
419+
EventType.EVENT_TYPE_CHILD_WORKFLOW_EXECUTION_STARTED,
420+
ChildWorkflowExecutionStartedEventAttributes.newBuilder()
421+
.setInitiatedEventId(5)
422+
.build())
423+
.addWorkflowTask()
424+
.add(
425+
EventType.EVENT_TYPE_REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_INITIATED,
426+
RequestCancelExternalWorkflowExecutionInitiatedEventAttributes.newBuilder().build())
427+
.addWorkflowTaskScheduled()
428+
.add(
429+
EventType.EVENT_TYPE_EXTERNAL_WORKFLOW_EXECUTION_CANCEL_REQUESTED,
430+
ExternalWorkflowExecutionCancelRequestedEventAttributes.newBuilder()
431+
.setInitiatedEventId(10)
432+
.build())
433+
.addWorkflowTaskScheduled()
434+
.addWorkflowTaskStarted();
435+
436+
TestListener listener = new TestListener();
437+
stateMachines = newStateMachines(listener);
438+
439+
h.handleWorkflowTask(stateMachines);
440+
List<ExecuteLocalActivityParameters> requests = stateMachines.takeLocalActivityRequests();
441+
assertEquals(1, requests.size());
442+
assertEquals("id1", requests.get(0).getActivityId());
443+
List<Command> commands = stateMachines.takeCommands();
444+
assertTrue(commands.isEmpty());
445+
}
346446
}

‎temporal-sdk/src/test/java/io/temporal/internal/statemachines/TestHistoryBuilder.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,18 +553,27 @@ private HistoryEvent newAttributes(EventType type, Object attributes) {
553553
result.setWorkflowExecutionUpdateCompletedEventAttributes(
554554
(WorkflowExecutionUpdateCompletedEventAttributes) attributes);
555555
break;
556+
case EVENT_TYPE_START_CHILD_WORKFLOW_EXECUTION_INITIATED:
557+
result.setStartChildWorkflowExecutionInitiatedEventAttributes(
558+
(StartChildWorkflowExecutionInitiatedEventAttributes) attributes);
559+
break;
560+
case EVENT_TYPE_REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_INITIATED:
561+
result.setRequestCancelExternalWorkflowExecutionInitiatedEventAttributes(
562+
(RequestCancelExternalWorkflowExecutionInitiatedEventAttributes) attributes);
563+
break;
564+
case EVENT_TYPE_EXTERNAL_WORKFLOW_EXECUTION_CANCEL_REQUESTED:
565+
result.setExternalWorkflowExecutionCancelRequestedEventAttributes(
566+
(ExternalWorkflowExecutionCancelRequestedEventAttributes) attributes);
567+
break;
556568

557569
case EVENT_TYPE_UNSPECIFIED:
558570
case EVENT_TYPE_WORKFLOW_EXECUTION_FAILED:
559571
case EVENT_TYPE_WORKFLOW_EXECUTION_TIMED_OUT:
560572
case EVENT_TYPE_WORKFLOW_EXECUTION_CANCEL_REQUESTED:
561573
case EVENT_TYPE_WORKFLOW_EXECUTION_CANCELED:
562-
case EVENT_TYPE_REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_INITIATED:
563574
case EVENT_TYPE_REQUEST_CANCEL_EXTERNAL_WORKFLOW_EXECUTION_FAILED:
564-
case EVENT_TYPE_EXTERNAL_WORKFLOW_EXECUTION_CANCEL_REQUESTED:
565575
case EVENT_TYPE_WORKFLOW_EXECUTION_TERMINATED:
566576
case EVENT_TYPE_WORKFLOW_EXECUTION_CONTINUED_AS_NEW:
567-
case EVENT_TYPE_START_CHILD_WORKFLOW_EXECUTION_INITIATED:
568577
case EVENT_TYPE_START_CHILD_WORKFLOW_EXECUTION_FAILED:
569578
case EVENT_TYPE_SIGNAL_EXTERNAL_WORKFLOW_EXECUTION_INITIATED:
570579
case EVENT_TYPE_SIGNAL_EXTERNAL_WORKFLOW_EXECUTION_FAILED:

‎temporal-sdk/src/test/java/io/temporal/workflow/activityTests/LocalActivityAfterCancelTest.java

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Copyright (C) 2022 Temporal Technologies, Inc. All Rights Reserved.
3+
*
4+
* Copyright (C) 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
5+
*
6+
* Modifications copyright (C) 2017 Uber Technologies, Inc.
7+
*
8+
* Licensed under the Apache License, Version 2.0 (the "License");
9+
* you may not use this material except in compliance with the License.
10+
* You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
21+
package io.temporal.workflow.activityTests;
22+
23+
import static org.junit.Assert.assertThrows;
24+
25+
import io.temporal.activity.LocalActivityOptions;
26+
import io.temporal.api.enums.v1.EventType;
27+
import io.temporal.api.enums.v1.ParentClosePolicy;
28+
import io.temporal.client.WorkflowClient;
29+
import io.temporal.client.WorkflowFailedException;
30+
import io.temporal.client.WorkflowStub;
31+
import io.temporal.failure.TemporalFailure;
32+
import io.temporal.testing.WorkflowReplayer;
33+
import io.temporal.testing.internal.SDKTestWorkflowRule;
34+
import io.temporal.workflow.*;
35+
import io.temporal.workflow.shared.TestActivities.TestActivitiesImpl;
36+
import io.temporal.workflow.shared.TestActivities.VariousTestActivities;
37+
import io.temporal.workflow.shared.TestWorkflows;
38+
import io.temporal.workflow.shared.TestWorkflows.TestWorkflow1;
39+
import java.time.Duration;
40+
import org.junit.Assert;
41+
import org.junit.Rule;
42+
import org.junit.Test;
43+
44+
public class LocalActivityAfterCancelTest {
45+
private final TestActivitiesImpl activitiesImpl = new TestActivitiesImpl();
46+
47+
@Rule
48+
public SDKTestWorkflowRule testWorkflowRule =
49+
SDKTestWorkflowRule.newBuilder()
50+
.setWorkflowTypes(TestLocalActivityRetry.class, BlockingWorkflow.class)
51+
.setActivityImplementations(activitiesImpl)
52+
.build();
53+
54+
@Test
55+
public void localActivityAfterChildWorkflowCanceled() {
56+
TestWorkflow1 workflowStub =
57+
testWorkflowRule.newWorkflowStubTimeoutOptions(TestWorkflow1.class);
58+
WorkflowClient.execute(workflowStub::execute, "sada");
59+
WorkflowStub.fromTyped(workflowStub).cancel();
60+
WorkflowFailedException exception =
61+
Assert.assertThrows(WorkflowFailedException.class, () -> workflowStub.execute("sada"));
62+
Assert.assertEquals(
63+
EventType.EVENT_TYPE_WORKFLOW_EXECUTION_CANCELED, exception.getWorkflowCloseEventType());
64+
}
65+
66+
@Test
67+
public void testLocalActivityAfterChildWorkflowCanceledReplay() {
68+
assertThrows(
69+
RuntimeException.class,
70+
() ->
71+
WorkflowReplayer.replayWorkflowExecutionFromResource(
72+
"testLocalActivityAfterCancelTest.json",
73+
LocalActivityAfterCancelTest.TestLocalActivityRetry.class));
74+
}
75+
76+
@WorkflowInterface
77+
public static class BlockingWorkflow implements TestWorkflows.TestWorkflowReturnString {
78+
@Override
79+
public String execute() {
80+
Workflow.await(() -> false);
81+
return "";
82+
}
83+
}
84+
85+
public static class TestLocalActivityRetry implements TestWorkflow1 {
86+
87+
@Override
88+
public String execute(String taskQueue) {
89+
try {
90+
ChildWorkflowOptions childOptions =
91+
ChildWorkflowOptions.newBuilder()
92+
.setWorkflowId(Workflow.getInfo().getWorkflowId() + "-child1")
93+
.setCancellationType(ChildWorkflowCancellationType.WAIT_CANCELLATION_REQUESTED)
94+
.setParentClosePolicy(ParentClosePolicy.PARENT_CLOSE_POLICY_REQUEST_CANCEL)
95+
.validateAndBuildWithDefaults();
96+
TestWorkflows.TestWorkflowReturnString child =
97+
Workflow.newChildWorkflowStub(
98+
TestWorkflows.TestWorkflowReturnString.class, childOptions);
99+
child.execute();
100+
} catch (TemporalFailure e) {
101+
if (CancellationScope.current().isCancelRequested()) {
102+
Workflow.newDetachedCancellationScope(
103+
() -> {
104+
VariousTestActivities act =
105+
Workflow.newLocalActivityStub(
106+
VariousTestActivities.class,
107+
LocalActivityOptions.newBuilder()
108+
.setStartToCloseTimeout(Duration.ofSeconds(5))
109+
.validateAndBuildWithDefaults());
110+
act.activity1(10);
111+
})
112+
.run();
113+
throw e;
114+
}
115+
}
116+
return "dsadsa";
117+
}
118+
}
119+
}

0 commit comments

Comments
 (0)