Skip to content

KAFKA-10362: When resuming Streams active task with EOS, the checkpoint file is deleted#9247

Merged
guozhangwang merged 4 commits intoapache:trunkfrom
DOJI45:KAFKA-10362
Oct 7, 2020
Merged

KAFKA-10362: When resuming Streams active task with EOS, the checkpoint file is deleted#9247
guozhangwang merged 4 commits intoapache:trunkfrom
DOJI45:KAFKA-10362

Conversation

@DOJI45
Copy link
Copy Markdown
Contributor

@DOJI45 DOJI45 commented Sep 2, 2020

What was the issue?
https://issues.apache.org/jira/browse/KAFKA-10362

Deleted the checkpoint file before the transition from SUSPENDED state to RESTORING state

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@mjsax mjsax added the streams label Sep 2, 2020
@DOJI45
Copy link
Copy Markdown
Contributor Author

DOJI45 commented Sep 6, 2020

@guozhangwang request you to please review PR

@guozhangwang
Copy link
Copy Markdown
Contributor

guozhangwang commented Sep 9, 2020

test this please

guozhangwang
guozhangwang reviewed Sep 9, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DOJI45 Thanks for the PR! Could you also add a unit test to complete the coverage? You can find some examples in StreamTaskTest or ProcessorStateManagerTest

Comment thread streams/src/main/java/org/apache/kafka/streams/processor/internals/ProcessorStateManager.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java Outdated
@guozhangwang
Copy link
Copy Markdown
Contributor

guozhangwang commented Sep 16, 2020

@DOJI45 Thanks for the PR! Could you also add a unit test to complete the coverage? You can find some examples in StreamTaskTest or ProcessorStateManagerTest

Seems this comment is not addressed yet?

@DOJI45
Copy link
Copy Markdown
Contributor Author

DOJI45 commented Sep 16, 2020

@DOJI45 Thanks for the PR! Could you also add a unit test to complete the coverage? You can find some examples in StreamTaskTest or ProcessorStateManagerTest

Seems this comment is not addressed yet?

Hi @guozhangwang

I wrote unit test cases in ProcessorStateManagerTest for the new method that I have added

Not sure if I can write unit test case in StreamTaskTest class, because functionality wise i won't be able to validate anything, as deleting checkpoint file is tested in ProcessorStateManagerTest, and coverage is taken care by the existing unit test cases.

Please suggest how to proceed?

@DOJI45 DOJI45 requested a review from guozhangwang September 16, 2020 17:28
@guozhangwang guozhangwang merged commit a8b5f5a into apache:trunk Oct 7, 2020
@guozhangwang
Copy link
Copy Markdown
Contributor

guozhangwang commented Oct 7, 2020

Merged to trunk, thanks @DOJI45

javierfreire pushed a commit to javierfreire/kafka that referenced this pull request Oct 8, 2020
@DOJI45 @javierfreire
KAFKA-10362: When resuming Streams active task with EOS, the checkpoi…
d94b837 
…nt file is deleted (apache#9247)

Deleted the checkpoint file before the transition from SUSPENDED state to RESTORING state

Reviewers: Guozhang Wang <wangguoz@gmail.com>
ijuma added a commit to confluentinc/kafka that referenced this pull request Oct 8, 2020
@ijuma
CONFLUENT: Sync from apache/kafka (8 October 2020)
83f1575 
* 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)
  ...
ableegoldman pushed a commit that referenced this pull request Feb 2, 2021
@DOJI45 @ableegoldman
KAFKA-10362: When resuming Streams active task with EOS, the checkpoi…
ba0f04b 
…nt file is deleted (#9247)

Deleted the checkpoint file before the transition from SUSPENDED state to RESTORING state

Reviewers: Guozhang Wang <wangguoz@gmail.com>
@ableegoldman
Copy link
Copy Markdown
Member

ableegoldman commented Feb 2, 2021

Cherrypicked to 2.6 for 2.6.2 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guozhangwang guozhangwang Awaiting requested review from guozhangwang

Assignees

No one assigned

Labels

streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DOJI45 @guozhangwang @ableegoldman @mjsax