Skip to content

Commit f02a1f7

Browse files
committed
Fix test failures
1 parent 9dc54a0 commit f02a1f7

7 files changed

Lines changed: 141 additions & 10 deletions

File tree

temporal-sdk/src/main/java/io/temporal/internal/worker/AsyncPoller.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public CompletableFuture<Void> shutdown(ShutdownManager shutdownManager, boolean
134134
return super.shutdown(shutdownManager, interruptTasks)
135135
.thenApply(
136136
(f) -> {
137-
if (!namespaceCapabilities.isGracefulPollShutdown()) {
137+
if (interruptTasks || !namespaceCapabilities.isGracefulPollShutdown()) {
138138
for (PollTaskAsync<T> asyncTaskPoller : asyncTaskPollers) {
139139
try {
140140
log.debug("Shutting down async poller: {}", asyncTaskPoller.getLabel());

temporal-sdk/src/main/java/io/temporal/internal/worker/BasePoller.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,14 @@ public CompletableFuture<Void> shutdown(ShutdownManager shutdownManager, boolean
6161
}
6262

6363
CompletableFuture<Void> pollExecutorShutdown;
64-
if (namespaceCapabilities.isGracefulPollShutdown()) {
64+
if (namespaceCapabilities.isGracefulPollShutdown() && !interruptTasks) {
6565
// When graceful poll shutdown is enabled, the server will complete outstanding polls with
6666
// empty responses after ShutdownWorker is called. We simply wait for polls to return.
6767
pollExecutorShutdown =
6868
shutdownManager.shutdownExecutor(
6969
pollExecutor, this + "#pollExecutor", Duration.ofSeconds(80));
7070
} else {
71-
// Old behaviour forcibly stops outstanding polls.
71+
// ShutdownNow and old servers forcibly stop outstanding polls.
7272
pollExecutorShutdown =
7373
shutdownManager.shutdownExecutorNow(
7474
pollExecutor, this + "#pollExecutor", Duration.ofSeconds(1));

temporal-sdk/src/main/java/io/temporal/internal/worker/ShutdownManager.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,18 @@ public CompletableFuture<Void> waitOnWorkerShutdownRequest(
149149
future.complete(null);
150150
}
151151
},
152-
scheduledExecutorService);
152+
this::executeOrRunDirect);
153153
return future;
154154
}
155155

156+
private void executeOrRunDirect(Runnable command) {
157+
try {
158+
scheduledExecutorService.execute(command);
159+
} catch (RejectedExecutionException e) {
160+
command.run();
161+
}
162+
}
163+
156164
@Override
157165
public void close() {
158166
scheduledExecutorService.shutdownNow();

temporal-sdk/src/main/java/io/temporal/internal/worker/WorkflowWorker.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ public CompletableFuture<Void> shutdown(ShutdownManager shutdownManager, boolean
214214
!interruptTasks
215215
&& !options.getDrainStickyTaskQueueTimeout().isZero()
216216
&& stickyTaskQueueName != null
217-
&& stickyQueueBalancer != null;
217+
&& stickyQueueBalancer != null
218+
&& !namespaceCapabilities.isGracefulPollShutdown();
218219

219220
CompletableFuture<Void> pollerShutdown =
220221
CompletableFuture.completedFuture(null)

temporal-sdk/src/test/java/io/temporal/internal/worker/GracefulPollShutdownTest.java

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,4 +147,100 @@ public synchronized String poll() {
147147

148148
shutdownManager.close();
149149
}
150+
151+
@Test(timeout = 10_000)
152+
public void shutdownNowInterruptsInflightPollWhenGraceful() throws Exception {
153+
NamespaceCapabilities capabilities = new NamespaceCapabilities();
154+
capabilities.setFromCapabilities(
155+
Capabilities.newBuilder().setWorkerPollCompleteOnShutdown(true).build());
156+
157+
AtomicReference<String> processedTask = new AtomicReference<>();
158+
CountDownLatch taskProcessedLatch = new CountDownLatch(1);
159+
ShutdownableTaskExecutor<String> taskExecutor =
160+
new ShutdownableTaskExecutor<String>() {
161+
@Override
162+
public void process(@Nonnull String task) {
163+
processedTask.set(task);
164+
taskProcessedLatch.countDown();
165+
}
166+
167+
@Override
168+
public boolean isShutdown() {
169+
return false;
170+
}
171+
172+
@Override
173+
public boolean isTerminated() {
174+
return false;
175+
}
176+
177+
@Override
178+
public CompletableFuture<Void> shutdown(
179+
ShutdownManager shutdownManager, boolean interruptTasks) {
180+
return CompletableFuture.completedFuture(null);
181+
}
182+
183+
@Override
184+
public void awaitTermination(long timeout, TimeUnit unit) {}
185+
};
186+
187+
CountDownLatch secondPollStarted = new CountDownLatch(1);
188+
CountDownLatch releasePoll = new CountDownLatch(1);
189+
190+
MultiThreadedPoller.PollTask<String> pollTask =
191+
new MultiThreadedPoller.PollTask<String>() {
192+
private int callCount = 0;
193+
194+
@Override
195+
public synchronized String poll() {
196+
callCount++;
197+
if (callCount == 1) {
198+
return "task-1";
199+
} else if (callCount == 2) {
200+
secondPollStarted.countDown();
201+
try {
202+
releasePoll.await();
203+
} catch (InterruptedException e) {
204+
Thread.currentThread().interrupt();
205+
return null;
206+
}
207+
return "task-2";
208+
}
209+
try {
210+
Thread.sleep(Long.MAX_VALUE);
211+
} catch (InterruptedException e) {
212+
Thread.currentThread().interrupt();
213+
}
214+
return null;
215+
}
216+
};
217+
218+
MultiThreadedPoller<String> poller =
219+
new MultiThreadedPoller<>(
220+
"test-identity",
221+
pollTask,
222+
taskExecutor,
223+
PollerOptions.newBuilder()
224+
.setPollerBehavior(new PollerBehaviorSimpleMaximum(1))
225+
.setPollThreadNamePrefix("test-poller")
226+
.build(),
227+
new NoopScope(),
228+
capabilities);
229+
230+
poller.start();
231+
232+
assertTrue("first task should be processed", taskProcessedLatch.await(5, TimeUnit.SECONDS));
233+
assertEquals("task-1", processedTask.get());
234+
assertTrue("second poll should start", secondPollStarted.await(5, TimeUnit.SECONDS));
235+
236+
ShutdownManager shutdownManager = new ShutdownManager();
237+
try {
238+
poller.shutdown(shutdownManager, true).get(5, TimeUnit.SECONDS);
239+
assertNotEquals(
240+
"task-2 should not be processed by shutdownNow", "task-2", processedTask.get());
241+
} finally {
242+
releasePoll.countDown();
243+
shutdownManager.close();
244+
}
245+
}
150246
}

temporal-sdk/src/test/java/io/temporal/worker/WorkerShutdownTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public void activeTaskQueueTypesEvaluatedAtShutdownTime() throws Exception {
9797
.thenReturn(
9898
WorkflowClientOptions.newBuilder()
9999
.setNamespace("test-ns")
100+
.setIdentity("test-worker")
100101
.validateAndBuildWithDefaults());
101102

102103
Scope metricsScope = new NoopScope();
@@ -113,7 +114,7 @@ public void activeTaskQueueTypesEvaluatedAtShutdownTime() throws Exception {
113114
metricsScope,
114115
runLocks,
115116
cache,
116-
false,
117+
true,
117118
wfThreadExecutor,
118119
Collections.emptyList(),
119120
Collections.emptyList(),
@@ -147,5 +148,8 @@ public void activeTaskQueueTypesEvaluatedAtShutdownTime() throws Exception {
147148
"ShutdownWorkerRequest heartbeat should report SHUTTING_DOWN",
148149
WorkerStatus.WORKER_STATUS_SHUTTING_DOWN,
149150
captor.getValue().getWorkerHeartbeat().getStatus());
151+
assertTrue(
152+
"ShutdownWorkerRequest sticky task queue should be derived from worker identity",
153+
captor.getValue().getStickyTaskQueue().startsWith("test-worker:"));
150154
}
151155
}

temporal-sdk/src/test/java/io/temporal/worker/shutdown/StickyWorkflowDrainShutdownTest.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import static org.junit.Assert.assertEquals;
44
import static org.junit.Assert.assertTrue;
55

6+
import io.temporal.api.workflowservice.v1.DescribeNamespaceRequest;
7+
import io.temporal.api.workflowservice.v1.DescribeNamespaceResponse;
68
import io.temporal.client.WorkflowClient;
79
import io.temporal.client.WorkflowStub;
810
import io.temporal.serviceclient.WorkflowServiceStubsOptions;
@@ -53,6 +55,7 @@ public StickyWorkflowDrainShutdownTest(PollerBehavior pollerBehaviorAutoscaling)
5355

5456
@Test
5557
public void testShutdown() throws InterruptedException {
58+
boolean gracefulPollShutdownSupported = isGracefulPollShutdownSupported();
5659
TestWorkflow1 workflow = testWorkflowRule.newWorkflowStub(TestWorkflow1.class);
5760
WorkflowClient.start(workflow::execute, null);
5861
testWorkflowRule.getTestEnvironment().shutdown();
@@ -62,10 +65,16 @@ public void testShutdown() throws InterruptedException {
6265
assertTrue(testWorkflowRule.getTestEnvironment().getWorkerFactory().isTerminated());
6366
System.out.println("Shutdown completed");
6467
long endTime = System.currentTimeMillis();
65-
assertTrue("Drain time should be respected", endTime - startTime > DRAIN_TIME.toMillis());
66-
// Workflow should complete successfully since the drain time is longer than the workflow
67-
// execution time
68-
assertEquals("Success", workflow.execute(null));
68+
WorkflowStub untyped = WorkflowStub.fromTyped(workflow);
69+
if (gracefulPollShutdownSupported) {
70+
assertTrue("Drain time should be skipped", endTime - startTime < DRAIN_TIME.toMillis());
71+
untyped.terminate("test cleanup");
72+
} else {
73+
assertTrue("Drain time should be respected", endTime - startTime > DRAIN_TIME.toMillis());
74+
// Workflow should complete successfully since the drain time is longer than the workflow
75+
// execution time.
76+
assertEquals("Success", untyped.getResult(String.class));
77+
}
6978
}
7079

7180
@Test
@@ -94,4 +103,17 @@ public String execute(String now) {
94103
return "Success";
95104
}
96105
}
106+
107+
private boolean isGracefulPollShutdownSupported() {
108+
DescribeNamespaceResponse response =
109+
testWorkflowRule
110+
.getWorkflowClient()
111+
.getWorkflowServiceStubs()
112+
.blockingStub()
113+
.describeNamespace(
114+
DescribeNamespaceRequest.newBuilder()
115+
.setNamespace(testWorkflowRule.getWorkflowClient().getOptions().getNamespace())
116+
.build());
117+
return response.getNamespaceInfo().getCapabilities().getWorkerPollCompleteOnShutdown();
118+
}
97119
}

0 commit comments

Comments
 (0)