KAFKA-10564: only process non-empty task directories when internally cleaning obsolete state stores#9373
Merged
guozhangwang merged 3 commits intoapache:trunkfrom Oct 7, 2020
Merged
Conversation
…lly cleaning obsolete state stores
ableegoldman
reviewed
Oct 6, 2020
Member
ableegoldman
left a comment
ableegoldman
left a comment
There was a problem hiding this comment.
Thanks for the quick patch! I love a good one-liner 😄
Can you include a test in StateDirectoryTest as well? I think it would be sufficient to start out with a single non-empty task directory and then verify that the first time cleanRemovedTasksCalledByCleanerThread() is called it will log this message + clear out everything except the lock file, but if it's invoked a second time, nothing is logged.
Member
|
By the way, John was so kind as to cherrypick the trunk PR that rearranged things in this method back to the 2.6 branch -- so we should be able to cherrypick this PR smoothly, no need for a separate one for 2.6 Looks like just a handful of unrelated flaky test failures in the builds, hopefully we'll have better luck on the next run |
Contributor
Author
|
Thanks for the review @ableegoldman! Added a unit test. And thanks @vvcephei for making it easier to cherry pick this fix back to 2.6. |
ableegoldman
approved these changes
Oct 6, 2020
Member
ableegoldman
left a comment
ableegoldman
left a comment
There was a problem hiding this comment.
LGTM!
Only known flaky tests in the build failure:
Build / JDK 11 / org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
Build / JDK 11 / org.apache.kafka.streams.integration.EosIntegrationTest.shouldNotViolateEosIfOneTaskFailsWithState[exactly_once_beta]
Build / JDK 15 / org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
Member
|
FYI this should be cherrypicked back to 2.6 once merged |
Contributor
|
Cherry-picked to 2.6. |
guozhangwang
pushed a commit
that referenced
this pull request
Oct 7, 2020
…cleaning obsolete state stores (#9373) Avoid continuous repeated logging by not trying to clean empty task directories, which are longer fully deleted during internal cleanup as of https://issues.apache.org/jira/browse/KAFKA-6647. Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
Contributor
Author
|
Thank you all! |
javierfreire
pushed a commit
to javierfreire/kafka
that referenced
this pull request
Oct 8, 2020
…cleaning obsolete state stores (apache#9373) Avoid continuous repeated logging by not trying to clean empty task directories, which are longer fully deleted during internal cleanup as of https://issues.apache.org/jira/browse/KAFKA-6647. Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Guozhang Wang <wangguoz@gmail.com>
ijuma
added a commit
to confluentinc/kafka
that referenced
this pull request
Oct 8, 2020
* commit '2804257fe221f37e5098bd': (67 commits) KAFKA-10562: Properly invoke new StateStoreContext init (apache#9388) MINOR: trivial cleanups, javadoc errors, omitted StateStore tests, etc. (apache#8130) KAFKA-10564: only process non-empty task directories when internally cleaning obsolete state stores (apache#9373) KAFKA-9274: fix incorrect default value for `task.timeout.ms` config (apache#9385) KAFKA-10362: When resuming Streams active task with EOS, the checkpoint file is deleted (apache#9247) KAFKA-10028: Implement write path for feature versioning system (KIP-584) (apache#9001) KAFKA-10402: Upgrade system tests to python3 (apache#9196) KAFKA-10186; Abort transaction with pending data with TransactionAbortedException (apache#9280) MINOR: Remove `TargetVoters` from `DescribeQuorum` (apache#9376) Revert "KAFKA-10469: Resolve logger levels hierarchically (apache#9266)" MINOR: Don't publish javadocs for raft module (apache#9336) KAFKA-9929: fix: add missing default implementations (apache#9321) KAFKA-10188: Prevent SinkTask::preCommit from being called after SinkTask::stop (apache#8910) KAFKA-10338; Support PEM format for SSL key and trust stores (KIP-651) (apache#9345) KAFKA-10527; Voters should not reinitialize as leader in same epoch (apache#9348) MINOR: Refactor unit tests around RocksDBConfigSetter (apache#9358) KAFKA-6733: Printing additional ConsumerRecord fields in DefaultMessageFormatter (apache#9099) MINOR: Annotate test BlockingConnectorTest as integration test (apache#9379) MINOR: Fix failing test due to KAFKA-10556 PR (apache#9372) KAFKA-10439: Connect's Values to parse BigInteger as Decimal with zero scale. (apache#9320) ...
Member
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Avoid continuous repeated logging by not trying to clean empty task directories, which are longer fully deleted during internal cleanup as of https://issues.apache.org/jira/browse/KAFKA-6647.
Committer Checklist (excluded from commit message)