Skip to content

KAFKA-7692: Fix ProduceStateManager SequenceNumber overflow.#5990

Merged
hachikuji merged 6 commits intoapache:trunkfrom
mingaliu:1202StateManagerSequenceId
Jan 25, 2019
Merged

KAFKA-7692: Fix ProduceStateManager SequenceNumber overflow.#5990
hachikuji merged 6 commits intoapache:trunkfrom
mingaliu:1202StateManagerSequenceId

Conversation

@mingaliu
Copy link
Copy Markdown
Contributor

@mingaliu mingaliu commented Dec 3, 2018

This is to fix Kafka 7692. The problem is at ProducerStateManager:Append(),
it has those lines:
updatedEntry.addBatch(epoch, lastSeq, lastOffset, lastSeq - firstSeq, lastTimestamp)
and
val firstOffset = lastOffset - (lastSeq - firstSeq)
However, lastSeq can wrap around and smaller than firstSeq. This will cause some exception later on (described in the JIRA 7692)

Test: Verified with local unitest and in our daily operation.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 3, 2018

Cc @hachikuji

@ijuma ijuma requested review from hachikuji and junrao December 8, 2018 17:34
Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The fix looks good. It would be helpful to have a test case while overflows the sequence number.

@mingaliu
Copy link
Copy Markdown
Contributor Author

@hachikuji Thanks for the comments. I updated the PR with unitest.

@mingaliu
Copy link
Copy Markdown
Contributor Author

Tested multiple times and succeed locally, the jenkins is due to other random test failure. @hachikuji , please review. Thanks!

@mingaliu
Copy link
Copy Markdown
Contributor Author

Please review or sign off, @junrao , @hachikuji , thanks!

Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the patch!

@hachikuji hachikuji merged commit e4b54a5 into apache:trunk Jan 25, 2019
hachikuji pushed a commit that referenced this pull request Jan 25, 2019
This patch fixes a few overflow issues with wrapping sequence numbers in the broker's producer state tracking. 

Reviewers: Jason Gustafson <jason@confluent.io>
hachikuji pushed a commit that referenced this pull request Jan 25, 2019
This patch fixes a few overflow issues with wrapping sequence numbers in the broker's producer state tracking. 

Reviewers: Jason Gustafson <jason@confluent.io>
hachikuji pushed a commit that referenced this pull request Jan 25, 2019
This patch fixes a few overflow issues with wrapping sequence numbers in the broker's producer state tracking. 

Reviewers: Jason Gustafson <jason@confluent.io>
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* ak/trunk:
  MINOR: Update usage of deprecated API (apache#6146)
  KAFKA-4217: Add KStream.flatTransform (apache#5273)
  MINOR: Update Gradle to 5.1.1 (apache#6160)
  KAFKA-3522: Generalize Segments (apache#6170)
  Added quotes around the class path (apache#4469)
  KAFKA-7837: Ensure offline partitions are picked up as soon as possible when shrinking ISR (apache#6202)
  MINOR: In the MetadataResponse schema, ignorable should be a boolean
  KAFKA-7838: Log leader and follower end offsets when shrinking ISR (apache#6168)
  KAFKA-5692: Change PreferredReplicaLeaderElectionCommand to use Admin… (apache#3848)
  MINOR: clarify why suppress can sometimes drop tombstones (apache#6195)
  MINOR: Upgrade ducktape to 0.7.5 (apache#6197)
  MINOR: Improve IntegrationTestUtils documentation (apache#5664)
  MINOR: upgrade to jdk8 8u202
  KAFKA-7693; Fix SequenceNumber overflow in producer (apache#5989)
  KAFKA-7692; Fix ProducerStateManager SequenceNumber overflow (apache#5990)
  MINOR: update copyright year in the NOTICE file. (apache#6196)
  KAFKA-7793: Improve the Trogdor command line. (apache#6133)
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…5990)

This patch fixes a few overflow issues with wrapping sequence numbers in the broker's producer state tracking. 

Reviewers: Jason Gustafson <jason@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants