You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2023/07/27 19:13:58 UTC
[GitHub] [kafka] dajac opened a new pull request, #14117: MINOR: Code cleanups in group-coordinator module
dajac opened a new pull request, #14117:
URL: https://github.com/apache/kafka/pull/14117
This patch does a few code cleanups in the group-coordinator module.
- It renames `Coordinator` to `CoordinatorShard`;
- It renames `ReplicatedGroupCoordinator` to `GroupCoordinatorShard`. I was never really happy with this name. The new name makes more sense to me;
- It removes `TopicPartition` from the `GroupMetadataManager`. It was only used in log messages. The log context already includes it so we don't have to log it again.
- It renames `assignors` to `consumerGroupAssignors`.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #14117:
URL: https://github.com/apache/kafka/pull/14117#issuecomment-1655195050
> can you point me to where the log context includes the topic partition info?
Sure. It is right [here](https://github.com/apache/kafka/blob/trunk/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java#L412).
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeffkbkim commented on pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "jeffkbkim (via GitHub)" <gi...@apache.org>.
jeffkbkim commented on PR #14117:
URL: https://github.com/apache/kafka/pull/14117#issuecomment-1654851021
can you point me to where the log context includes the topic partition info?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jolshan merged pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan merged PR #14117:
URL: https://github.com/apache/kafka/pull/14117
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jolshan commented on a diff in pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on code in PR #14117:
URL: https://github.com/apache/kafka/pull/14117#discussion_r1277712647
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java:
##########
@@ -1014,7 +1013,7 @@ public void onHighWatermarkUpdated(
* @param processor The event processor.
* @param partitionWriter The partition writer.
* @param loader The coordinator loader.
- * @param coordinatorBuilderSupplier The coordinator builder.
+ * @param coordinatorShardBuilderSupplier The coordinator builder.
Review Comment:
nit: this changed the spacing of the params. Do we want to line them up?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jeffkbkim commented on a diff in pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "jeffkbkim (via GitHub)" <gi...@apache.org>.
jeffkbkim commented on code in PR #14117:
URL: https://github.com/apache/kafka/pull/14117#discussion_r1277519026
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorBuilderSupplier.java:
##########
@@ -17,14 +17,14 @@
package org.apache.kafka.coordinator.group.runtime;
/**
- * Supplies a {@link CoordinatorBuilder} to the {@link CoordinatorRuntime}.
+ * Supplies a {@link CoordinatorShardBuilder} to the {@link CoordinatorRuntime}.
*
* @param <S> The type of the coordinator.
* @param <U> The record type.
*/
-public interface CoordinatorBuilderSupplier<S extends Coordinator<U>, U> {
+public interface CoordinatorBuilderSupplier<S extends CoordinatorShard<U>, U> {
Review Comment:
should this be CoordinatorShardBuilderSupplier?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on a diff in pull request #14117: MINOR: Code cleanups in group-coordinator module
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #14117:
URL: https://github.com/apache/kafka/pull/14117#discussion_r1277523528
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorBuilderSupplier.java:
##########
@@ -17,14 +17,14 @@
package org.apache.kafka.coordinator.group.runtime;
/**
- * Supplies a {@link CoordinatorBuilder} to the {@link CoordinatorRuntime}.
+ * Supplies a {@link CoordinatorShardBuilder} to the {@link CoordinatorRuntime}.
*
* @param <S> The type of the coordinator.
* @param <U> The record type.
*/
-public interface CoordinatorBuilderSupplier<S extends Coordinator<U>, U> {
+public interface CoordinatorBuilderSupplier<S extends CoordinatorShard<U>, U> {
Review Comment:
yeah, that makes sense.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org