Let activities heartbeat during worker shutdown#2903
Conversation
maciejdudko
left a comment
There was a problem hiding this comment.
Hi @baekgyu-kim, thank you for your contribution! It's great to see someone taking on these long standing issues. However, this is not the right implementation.
There should be a new worker option to enable heartbeating during shutdown, It should default to disabled, and when disabled, the behavior should be identical to existing behavior for backward compatibility purposes.
When the option is enabled, the heartbeat behavior should be identical to what happens during normal heartbeat when the worker is not shutting down. There should be no additional code path that calls sendHeartbeatRequest a different way, the existing mechanism should be used. The way to achieve that is to modify SyncActivityWorker.shutdown so that heartbeatExecutor.shutdown is only called after all outstanding activity tasks have finished executing.
If you need assistance with implementation, feel free to reach out on community Slack, either message me directly or post on #java-sdk channel.
|
Hi @maciejdudko, It now adds an experimental Whenever you have a chance, I'd appreciate another look. Thanks again! |
What was changed
heartbeatExecutoralready shut down),HeartbeatContextImpl.heartbeat()now emits the heartbeat to the server before throwingActivityWorkerShutdownException(previously: thrown without sending).heartbeat()cannot flood the server.ActivityWorkerShutdownExceptionis always thrown, so a transient failure cannot mask the shutdown signal.ActivityWorkerShutdownExceptionJavadoc updated accordingly.Why?
heartbeat()threw immediately without contacting the server → no heartbeat during theawaitTerminationgrace period → server times the activity out and retries it → duplicate executions, despite the worker deliberately giving the activity time to finish.Checklist
Closes Add the ability to keep heartbeating while the worker is shutting down #2075
How was this tested:
New
HeartbeatContextImplTestcases:Any docs updates needed?
ActivityWorkerShutdownExceptionJavadoc.