-
Notifications
You must be signed in to change notification settings - Fork 7
Improve GrpcWorker shutdown #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Javier Aliaga <javier@diagrid.io>
*/ | ||
public void start() { | ||
new Thread(this::startAndBlock).start(); | ||
this.workerThread = new Thread(this::startAndBlock); | ||
this.workerThread.start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real change
@@ -39,6 +40,7 @@ public final class DurableTaskGrpcWorker implements AutoCloseable { | |||
private final TaskHubSidecarServiceBlockingStub sidecarClient; | |||
private final boolean isExecutorServiceManaged; | |||
private volatile boolean isNormalShutdown = false; | |||
private Thread workerThread; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real change
* it is the responsibility of the supplier to take care of them. | ||
*/ | ||
public void close() { | ||
this.workerThread.interrupt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real change
Signed-off-by: Javier Aliaga <javier@diagrid.io>
ef61c25
to
d911e00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx Javi!
Issue describing the changes in this PR
resolves previous
TODO
Interrupt worker thread on stop
Pull request checklist
CHANGELOG.md
Additional information
Additional PR information