Skip to content

KAFKA-12376: Apply atomic append to the log#10253

Merged
mumrah merged 4 commits intoapache:trunkfrom
jsancio:kafka-12376-use-atomic-append
Mar 4, 2021
Merged

KAFKA-12376: Apply atomic append to the log#10253
mumrah merged 4 commits intoapache:trunkfrom
jsancio:kafka-12376-use-atomic-append

Conversation

@jsancio
Copy link
Copy Markdown
Member

@jsancio jsancio commented Mar 3, 2021

Append to the log in one batch when handling:

  1. Client quota changes
  2. Configuration changes
  3. Feature changes
  4. Topic creation

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

Copy link
Copy Markdown
Member

@mumrah mumrah 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 patch @jsancio! Looks like the RaftClient already had the atomic batch method, so it looks like we're just exposing that up to the controller "manager" classes?

Comment thread metadata/src/main/java/org/apache/kafka/metalog/LocalLogManager.java Outdated
Comment thread metadata/src/main/java/org/apache/kafka/controller/ControllerResult.java Outdated
@mumrah mumrah added the kraft label Mar 3, 2021
Comment thread metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java Outdated
Comment thread metadata/src/main/java/org/apache/kafka/metalog/MetaLogManager.java
Copy link
Copy Markdown
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

Left one non-trivial comment. After that and @tombentley's comments I think this LGTM

Comment thread metadata/src/main/java/org/apache/kafka/controller/FeatureControlManager.java Outdated
Copy link
Copy Markdown
Member Author

@jsancio jsancio 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 review @mumrah @tombentley

Comment thread metadata/src/main/java/org/apache/kafka/controller/FeatureControlManager.java Outdated
Comment thread metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java Outdated
@mumrah mumrah merged commit 96a2b7a into apache:trunk Mar 4, 2021
hachikuji added a commit that referenced this pull request Mar 4, 2021
Topic deletions should be atomic. This fixes a build error caused by merging of both #10253 and #10184 at about the same time. 

Reviewers: David Arthur <mumrah@gmail.com>
mumrah pushed a commit that referenced this pull request Mar 4, 2021
Topic deletions should be atomic. This fixes a build error caused by merging of both #10253 and #10184 at about the same time. 

Reviewers: David Arthur <mumrah@gmail.com>
@jsancio jsancio deleted the kafka-12376-use-atomic-append branch March 9, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants