KAFKA-10792 (2.5 backport): Prevent source task shutdown from blocking herder thread (#9669) #9880
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
…apache#9669) Changes the `WorkerSourceTask` class to only call `SourceTask::stop` from the task thread when the task is actually stopped (via `Source:task::close` just before `WorkerTask::run` completes), and only if an attempt has been made to start the task (which will not be the case if it was created in the paused state and then shut down before being started). This prevents `SourceTask::stop` from being indirectly invoked on the herder's thread, which can have adverse effects if the task is unable to shut down promptly. Unit tests are tweaked where necessary to account for this new logic, which covers some edge cases mentioned in PR apache#5020 that were unaddressed up until now. The existing integration tests for blocking connectors are expanded to also include cases for blocking source and sink tasks. Full coverage of every source/sink task method is intentionally omitted from these expanded tests in order to avoid inflating test runtime (each one adds an extra 5 seconds at minimum) and because the tests that are added here were sufficient to reproduce the bug with source task shutdown. Author: Chris Egerton <chrise@confluent.io> Reviewers: Nigel Liang <nigel@nigelliang.com>, Tom Bentley <tbentley@redhat.com>, Randall Hauch <rhauch@gmail.com>
|
I'm trying to get the builds to run properly, but I did pull locally and the build of that local branch does pass. |
|
There seem to be environmental build issues; the builds are failing with "JDK" unknown and "maven" unknown. I'm going to go ahead and merge this PR to the |
|
Thanks Randall! |
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.
Backports the changes from #9669.
Changes the
WorkerSourceTaskclass to only callSourceTask::stopfrom the task thread when the task is actually stopped (viaSource:task::closejust beforeWorkerTask::runcompletes), and only if an attempt has been made to start the task (which will not be the case if it was created in the paused state and then shut down before being started). This preventsSourceTask::stopfrom being indirectly invoked on the herder's thread, which can have adverse effects if the task is unable to shut down promptly.Unit tests are tweaked where necessary to account for this new logic, which covers some edge cases mentioned in PR #5020 that were unaddressed up until now.
The existing integration tests for blocking connectors added later in 2.6 are backported here to provide cases for blocking source and sink tasks. Full coverage of every source/sink task method is intentionally omitted from these expanded tests in order to avoid inflating test runtime (each one adds an extra 5 seconds at minimum) and because the tests that are added here were sufficient to reproduce the bug with source task shutdown.
Committer Checklist (excluded from commit message)