-
Notifications
You must be signed in to change notification settings - Fork 14.5k
MINOR: Add Append KRaft Version Record to BatchAccumulator #18956
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
Conversation
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 changes @nihyxx. Just some minor comments.
public void appendKRaftVersionRecord( | ||
KRaftVersionRecord kraftVersionRecord, | ||
long currentTimestamp | ||
) { |
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.
We indent by 4 spaces in Java code.
public void appendKRaftVersionRecord(
KRaftVersionRecord kraftVersionRecord,
long currentTimestamp
) {
appendControlMessages((baseOffset, epoch, compression, buffer) -> | ||
MemoryRecords.withKRaftVersionRecord( | ||
baseOffset, | ||
currentTimestamp, | ||
epoch, | ||
buffer, | ||
kraftVersionRecord | ||
) | ||
); |
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.
We indent by 4 spaces in Java code.
appendControlMessages((baseOffset, epoch, compression, buffer) ->
MemoryRecords.withKRaftVersionRecord(
baseOffset,
currentTimestamp,
epoch,
buffer,
kraftVersionRecord
)
);
Mockito.when(memoryPool.tryAllocate(maxBatchSize)) | ||
.thenReturn(buffer); | ||
|
||
BatchAccumulator<String> acc = buildAccumulator( | ||
leaderEpoch, | ||
baseOffset, | ||
lingerMs, | ||
maxBatchSize | ||
); |
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.
We indent by 4 spaces in Java code.
Mockito.when(memoryPool.tryAllocate(maxBatchSize))
.thenReturn(buffer);
BatchAccumulator<String> acc = buildAccumulator(
leaderEpoch,
baseOffset,
lingerMs,
maxBatchSize
);
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
This PR has been closed since it has not had any activity in 120 days. If you feel like this |
What
This minor PR adds a function to append the KRaft Version Record in BatchAccumulator. This is needed to implement KRaft upgrade (https://issues.apache.org/jira/browse/KAFKA-16538).
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)