hachikuji commented on code in PR #14213:
URL: https://github.com/apache/kafka/pull/14213#discussion_r1296563672
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -2653,11 +2652,11 @@ private boolean shouldFireLeaderChange(LeaderAndEpoch
leaderAndEpoch) {
}
private void maybeFireLeaderChange(LeaderAndEpoch leaderAndEpoch, long
epochStartOffset) {
- // If this node is becoming the leader, then we can fire
`handleClaim` as soon
+ // If this node is becoming the leader, then we can fire
`handleLeaderChange` as soon
Review Comment:
Maybe useful to mention the point about the high watermark as well since it
is a little subtle.
##########
raft/src/main/java/org/apache/kafka/raft/internals/RecordsBatchReader.java:
##########
@@ -117,14 +117,8 @@ private void ensureOpen() {
}
private Optional<Batch<T>> nextBatch() {
- while (iterator.hasNext()) {
- Batch<T> batch = iterator.next();
-
- if (batch.records().isEmpty()) {
- lastReturnedOffset = batch.lastOffset();
- } else {
- return Optional.of(batch);
- }
+ if (iterator.hasNext()) {
Review Comment:
We're testing this change indirectly, but perhaps it is useful to have an
explicit test in `RecordsBatchReaderTest` as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
