vamossagar12 commented on a change in pull request #9756: URL: https://github.com/apache/kafka/pull/9756#discussion_r556429212
########## 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: @hachikuji , @jsancio I agree to the points. I am slightly confused on the advantage of letting the config to be set higher than maxBatchSize(1MB) and then limiting the effective batch size to the minimum of `maxUnflushedBytes` and `maxBatchSize`. Won't it lead to confusion in terms of usage? Infact, I have followed a similar approach in the PR in a different way. What I have done is restricting the value of this new config within a range of 0(which is wrong, need to change) to maxBatchSize - 1: ``` .define(QUORUM_FLUSH_MIN_SIZE_BYTES_CONFIG, ConfigDef.Type.INT, 1024, between(0, (1024 * 1024) - 1), ConfigDef.Importance.MEDIUM, QUORUM_FLUSH_MIN_SIZE_BYTES_DOC); ``` I feel this way, what the config provides and what actually happens remains the same and effectively, this will be equivalent to `min(maxBatchsize, maxUnflushedBytes)`. Let me know your thoughts plz. ---------------------------------------------------------------- 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