SAMZA-2510: Incorrect shutdown status due to race between runloop and process callback thread #1344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ead and process callback thread
samza-core/src/main/java/org/apache/samza/container/RunLoop.java
Outdated
Show resolved
Hide resolved
cameronlee314
approved these changes
Apr 15, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Symptom: A race between the process callback thread and the runloop thread can result in incorrect shutdown status for shutdown triggered by end of stream.
Issue: Currently runloop performs message choosing and dispatching in a loop indefinitely until interrupted by a shutdown request or an exception during processing. In async mode, message completion is notified by the process callback thread using
onComplete()andonFailure()to represent corresponding success and failure. The failure callback updates the exception code within the runloop to notify processing failures. Similarly, shutdown can be requested by user code or end of stream messages through various scopes(task level, container level). These requests are notified to the runloop through shutdownNow flag.Currently as long as shutdownNow flag is not set, the exception code is promptly seen by the runloop and propagated upstream correctly. However, if a shutdown triggered by end of stream is requested, runloop doesn't check the exception code before exiting.
Changes: Make sure we check the exception code before we exit. Also, set the exception code on process failure before updating the state to done to promptly signal runloop about failure.
Tests: Added unit tests to check if the exception is thrown even if the shutdown is requested ahead of callback failure.
API Changes: None
Usage Instructions: None
Upgrade Instructions: None