-
Notifications
You must be signed in to change notification settings - Fork 467
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
Shutdown all record processors in an orderly fashion when we shutdown the JVM #39
Comments
I agree, this functionality is needed. See line 325 on Worker.java Line 325 in c6e393c
For example, if you catch TERM signal and stop your worker thread; worker shuts itself down and the lease coordinator. So the current running RecordProcessor classes will fail upon calling recordProcessorCheckpointer.checkpoint() method due to missing lease.
After exiting the while loop, and just before calling "leaseCoordinator.stop()" worker thread should tell all running threads, that it is exiting. |
I'm currently working to add support for orderly shutdown right now. I've put some comments in #79. I don't currently have a release date yet, but I do hope to have it soon. |
Graceful shutdown was added in Release 1.7.1. This doesn't add shutdown hooks for the JVM so you would need to add those yourself. Feel free to reopen if you have any other questions. |
* Add configurable max initialization attempts * Add maxInitializationAttempts to unit test setup
We often use JVM shutdown hook to make sure the processing pipeline is flushed in an orderly server shutdown.
For a record processor, what needs to be done in such case is pretty much the same as handling the "TERMINATE" command, i.e., finish the processing and checkpoint. For the current KCL design, however, to implement the JVM shutdown hook needs to manually keep track of all record processors and issue "TERMINATE" ourselves.
Ideally the Worker class's shutdown() method should take care of this, so we only need to call worker.shutdown(), but right now, it doesn't do that, and what it does seems to be no more than excluding itself from lease coordination.
The text was updated successfully, but these errors were encountered: