AndrewJSchofield commented on code in PR #23245:
URL: https://github.com/apache/kafka/pull/23245#discussion_r3854481062
##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -1231,7 +1252,7 @@ static void
validateTotalNumberOfPartitions(CreateTopicsRequestData request, int
} else {
totalPartitions += topic.assignments().size();
}
- if (totalPartitions > MAX_PARTITIONS_PER_BATCH) {
+ if (totalPartitions > maxRecordsPerBatch) {
throw new PolicyViolationException("Excessively large number
of partitions per request.");
Review Comment:
This exception message is really not that elegant. It sounds like the
configuration for the number of partitions per request is excessive, but
actually it's just complaining about an individual request. How about something
like "Too many partitions in request".
##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -1562,7 +1583,7 @@ void handleDirectoriesCordoned(
ControllerResult<ElectLeadersResponseData>
electLeaders(ElectLeadersRequestData request) {
ElectionType electionType = electionType(request.electionType());
- List<ApiMessageAndVersion> records =
BoundedList.newArrayBacked(MAX_RECORDS_PER_USER_OP);
+ List<ApiMessageAndVersion> records =
BoundedList.newArrayBacked(maxRecordsPerBatch);
Review Comment:
Just an observation from reading an unfamiliar area of code. This is going
to result in a `BoundedListTooLongException` if the list capacity is exceeded,
I think. Any reason why `validateTotalNumberOfPartitions` is taking a different
path?
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -247,6 +247,7 @@ class ControllerServer(
setQuorumFeatures(quorumFeatures).
setDefaultReplicationFactor(config.defaultReplicationFactor.toShort).
setDefaultNumPartitions(config.numPartitions.intValue()).
+ setMaxRecordsPerBatch(config.controllerMaxRecordsPerBatch).
Review Comment:
nit: Any reason why this config is not added at the end, preserving the
order after `controllerPerformanceAlwaysLogThresholdMs`?
--
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]
