-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
KAFKA-10198: guard against recycling dirty state #8924
KAFKA-10198: guard against recycling dirty state #8924
Conversation
/** | ||
* You must commit a task and checkpoint the state manager before closing as this will release the state dir lock | ||
*/ | ||
private void close(final boolean clean) { |
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 diff turned out a bit awkward, basically I just factored this check out into a separate method that we should call at the beginning of both flavors of clean close
Retest this please. |
LGTM. |
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.
Thanks for the fix, and for the test coverage, @ableegoldman !
Two unrelated test failures: |
We just needed to add the check in StreamTask#closeClean to closeAndRecycleState as well. I also renamed closeAndRecycleState to closeCleanAndRecycleState to drive this point home: it needs to be clean. This should be cherry-picked back to the 2.6 branch Reviewers: Matthias J. Sax <matthias@confluent.io>, John Roesler <john@confluent.io>, Guozhang Wang <wangguoz@gmail.com>,
Merged to trunk and cherry-picked to 2.6 |
* 'trunk' of github.com:apache/kafka: KAFKA-10180: Fix security_config caching in system tests (apache#8917) KAFKA-10173: Fix suppress changelog binary schema compatibility (apache#8905) KAFKA-10166: always write checkpoint before closing an (initialized) task (apache#8926) MINOR: Rename SslTransportLayer.State."NOT_INITALIZED" enum value to "NOT_INITIALIZED" MINOR: Update Scala to 2.13.3 (apache#8931) KAFKA-9076: support consumer sync across clusters in MM 2.0 (apache#7577) MINOR: Remove Diamond and code code Alignment (apache#8107) KAFKA-10198: guard against recycling dirty state (apache#8924)
We just needed to add the check in
StreamTask#closeClean
tocloseAndRecycleState
as well. I also renamedcloseAndRecycleState
tocloseCleanAndRecycleState
to drive this point home: it needs to be clean.This should be cherry-picked back to the 2.6 branch