josefk31 commented on code in PR #21028:
URL: https://github.com/apache/kafka/pull/21028#discussion_r2732967344
##########
raft/src/main/java/org/apache/kafka/raft/QuorumConfig.java:
##########
@@ -117,7 +125,9 @@ public class QuorumConfig {
.define(QUORUM_LINGER_MS_CONFIG, INT, DEFAULT_QUORUM_LINGER_MS,
atLeast(0), MEDIUM, QUORUM_LINGER_MS_DOC)
.define(QUORUM_REQUEST_TIMEOUT_MS_CONFIG, INT,
DEFAULT_QUORUM_REQUEST_TIMEOUT_MS, atLeast(0), MEDIUM,
QUORUM_REQUEST_TIMEOUT_MS_DOC)
.define(QUORUM_RETRY_BACKOFF_MS_CONFIG, INT,
DEFAULT_QUORUM_RETRY_BACKOFF_MS, atLeast(0), LOW, QUORUM_RETRY_BACKOFF_MS_DOC)
- .define(QUORUM_AUTO_JOIN_ENABLE_CONFIG, BOOLEAN,
DEFAULT_QUORUM_AUTO_JOIN_ENABLE, LOW, QUORUM_AUTO_JOIN_ENABLE_DOC);
+ .define(QUORUM_AUTO_JOIN_ENABLE_CONFIG, BOOLEAN,
DEFAULT_QUORUM_AUTO_JOIN_ENABLE, LOW, QUORUM_AUTO_JOIN_ENABLE_DOC)
+ .define(QUORUM_FETCH_SNAPSHOT_SIZE_MAX_BYTES_CONFIG, INT,
DEFAULT_QUORUM_FETCH_SNAPSHOT_SIZE_MAX_BYTES, atLeast(0), LOW,
QUORUM_FETCH_SNAPSHOT_SIZE_MAX_BYTES_DOC)
+ .define(QUORUM_FETCH_SIZE_MAX_BYTES_CONFIG, INT,
DEFAULT_QUORUM_FETCH_SIZE_MAX_BYTES, atLeast(0), LOW,
QUORUM_FETCH_SIZE_MAX_BYTES_DOC);
Review Comment:
Interesting question! Given that we will always return at least 1 batch we
_could_ support `0` values. After giving it some thought I am hesitant to do
so. I assume that most users will stick with the default or go higher / lower
depending on network conditions.
1. Semantically, it would be confusing to someone reading about this
configuration if it were possible to set to `0`. "Oh it's set to 0, that seems
like a mistake!"? But admittedly you could say "But setting it to N doesn't
really mean setting it to N, it's `max(N, sizeOf(nextBatch))`" so having the
possibility of `0` might break peoples assumptions about the value which is
maybe good?
2. If we assume that most won't touch this value or have a specific
meaningful non-zero intended value it is more likely that a `0` value is a
mistake/bug. Consider following example. Many programming languages will
automatically `0` integers - for example Golang. If a configuration file for
the cluster is generated in `golang` it is possible that there could be a bug
which doesn't initiate an integer and sets the configuration to `0`. A counter
point to this is that you can also have a bug where the value is `1`, `2`,
`3`... or some other small number. But I think the fact that integers are `0`
initialized makes it more likely that `0` would be an error rather than
intentional.
3. If someone did just want to, for whatever reason, say "please send at
most N bytes or the entire batch" they could get effectively the same effect by
setting `1`.
4. The other configuration will always have a min of `1` byte (admittedly a
pathological, yet correct case) so we keep them consistent with each other.
I think (1) is kinda ambivalent but (2) and (3) combined are a "legit"
reason for keeping the allowed value at `1`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
