Conversation
4462301 to
52437d7
Compare
52437d7 to
97a247d
Compare
… anticipate merge conflict
| .getNamespaceInfo() | ||
| .getCapabilities() | ||
| .getWorkerPollCompleteOnShutdown()) { | ||
| namespaceCapabilities.setGracefulPollShutdown(true); |
There was a problem hiding this comment.
Instead of calling namespaceCapbilities.set* for each field, could we have a generic namespaceCapabilities.populateFrom(describeNamespaceResponse), that way we don't need a separate if statement for each capability we want to capture?
| // When graceful poll shutdown is enabled, the server will complete outstanding polls with | ||
| // empty responses after ShutdownWorker is called. We simply wait for polls to return. | ||
| pollExecutorShutdown = | ||
| shutdownManager.shutdownExecutorUntimed(pollExecutor, this + "#pollExecutor"); |
There was a problem hiding this comment.
Can this potentially hang forever, if the server never responds?
There was a problem hiding this comment.
I don't believe so, since new polls should not get initiated... but I added a generous timeout just in case.
yuandrew
left a comment
There was a problem hiding this comment.
Overall LGTM, looks like there's a CI issue with GracefulPollShutdownTest.java
2a3ef46 to
b14918b
Compare
yuandrew
left a comment
There was a problem hiding this comment.
Sorry blocking this for now, actively investigating a potential bug in the Core implementation
What was changed
knows the worker is going away and can stop dispatching tasks to it.
interrupting them. This prevents tasks that were dispatched to the worker just before shutdown from being lost.
Why?
Prevents possible task loss during the shutdown process. Equivalent to: temporalio/sdk-core#1122
Checklist
Closes
How was this tested:
Added test, existing tests.
Any docs updates needed?