jsancio commented on a change in pull request #9756:
URL: https://github.com/apache/kafka/pull/9756#discussion_r555460647
##########
File path:
raft/src/main/java/org/apache/kafka/raft/internals/BatchAccumulator.java
##########
@@ -37,6 +37,7 @@
private final Time time;
private final SimpleTimer lingerTimer;
private final int lingerMs;
+ private final int minFlushSize;
Review comment:
I think it is important to agree on the current (`trunk`) behavior of
`BatchAccumulator`. It is my understanding that the `BatchAccumulator` creates
an in memory batch when `append` has been called enough times with enough data
such that the sum of records sizes is greater than `maxBatchSize`
(https://github.com/apache/kafka/pull/9756/files#diff-b7a2129b03764fbafab69c81e985d8ac6006d55f95307f0e21c70fb4750f61b5R145-R148)
**OR** the first un-drained `append` is `lingerMs` old
(https://github.com/apache/kafka/pull/9756/files#diff-b7a2129b03764fbafab69c81e985d8ac6006d55f95307f0e21c70fb4750f61b5R278).
I see two issues with the current implementation (`trunk`):
1. The draining thread will not call `drain`, even if `completed` is not
empty, until `lingerMs` has expired
(https://github.com/apache/kafka/pull/9756/files#diff-b7a2129b03764fbafab69c81e985d8ac6006d55f95307f0e21c70fb4750f61b5R254-R255).
Can this issue be address by checking if `completed` is non empty in
`timeUntilDrain`?
2. The KafkaRaftClient doesn't expose a way to configure the
`BatchAccumulator`'s `maxBatchSize`.
Can this issue be address by adding this configuration to `RaftConfig` and
the second constructor to `KafkaRaftClient`
(https://github.com/apache/kafka/pull/9756/files#diff-1da15c51e641ea46ea5c86201ab8f21cfee9e7c575102a39c7bae0d5ffd7de39R179)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
