dajac commented on code in PR #21555:
URL: https://github.com/apache/kafka/pull/21555#discussion_r2845107767
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorConfig.java:
##########
@@ -80,9 +80,14 @@ public class GroupCoordinatorConfig {
public static final int GROUP_COORDINATOR_APPEND_LINGER_MS_DEFAULT = -1;
public static final String GROUP_COORDINATOR_NUM_THREADS_CONFIG =
"group.coordinator.threads";
- public static final String GROUP_COORDINATOR_NUM_THREADS_DOC = "The number
of threads used by the group coordinator.";
+ public static final String GROUP_COORDINATOR_NUM_THREADS_DOC = "The number
of threads used by the group coordinator for processing requests.";
public static final int GROUP_COORDINATOR_NUM_THREADS_DEFAULT = 4;
+ public static final String GROUP_COORDINATOR_NUM_EXECUTOR_THREADS_CONFIG =
"group.coordinator.executor.threads";
+ public static final String GROUP_COORDINATOR_NUM_EXECUTOR_THREADS_DOC =
"The number of executor threads used by the group coordinator for " +
+ "updating the list of topics for regex subscriptions and metadata
changes and offloaded assignments.";
Review Comment:
> The number of executor threads used by the group coordinator for
processing background/asynchronous tasks such as resolving regular expressions
and computing assignments.
For the computing assignment part, we could mention the related config too
when we add it.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorConfig.java:
##########
@@ -80,9 +80,14 @@ public class GroupCoordinatorConfig {
public static final int GROUP_COORDINATOR_APPEND_LINGER_MS_DEFAULT = -1;
public static final String GROUP_COORDINATOR_NUM_THREADS_CONFIG =
"group.coordinator.threads";
- public static final String GROUP_COORDINATOR_NUM_THREADS_DOC = "The number
of threads used by the group coordinator.";
+ public static final String GROUP_COORDINATOR_NUM_THREADS_DOC = "The number
of threads used by the group coordinator for processing requests.";
public static final int GROUP_COORDINATOR_NUM_THREADS_DEFAULT = 4;
+ public static final String GROUP_COORDINATOR_NUM_EXECUTOR_THREADS_CONFIG =
"group.coordinator.executor.threads";
Review Comment:
Thinking about it, I find the name of the config not very good, mainly
because it is not clear why it does based on the name and it is not clear how
it differs from `group.coordinator.threads` too. I wonder whether we should use
something like `group.coordinator.backgroup.threads`. @squah-confluent
@dongnuo123 Thoughts?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorService.java:
##########
@@ -276,7 +276,7 @@ public GroupCoordinatorService build() {
.withSerializer(new GroupCoordinatorRecordSerde())
.withCompression(Compression.of(config.offsetTopicCompressionType()).build())
.withAppendLingerMs(config.appendLingerMs())
- .withExecutorService(Executors.newSingleThreadExecutor())
+
.withExecutorService(Executors.newFixedThreadPool(config.numExecutorThreads()))
Review Comment:
While we are here, would it be possible to give a proper name to the threads
used here? We could follow the name we use for the config so administrators can
connect the dots.
--
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]
