Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan merged PR #14387: URL: https://github.com/apache/kafka/pull/14387 -- 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-unsubscr...@kafka.apache.

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1820273369 Build looks good now. I will merge. -- 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 spe

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1813664706 @jolshan the other test failures look unrelated -- 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

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1804890304 Hmm tests are 😅 I see a lot of `java.util.concurrent.ExecutionException: java.lang.IllegalArgumentException: CoordinatorMetrics must be set.` Did we miss this somewhere? -- T

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1802757018 Something very strange is up with this build. I will try again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use th

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1385317000 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ## @@ -120,6 +121,9 @@ import static org.apache.kafka.coordinator.g

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1385307246 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -160,11 +161,14 @@ public GroupCoordinatorShard build() {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1799261509 > Also @jeffkbkim can you check the build? Seems like something is off. thanks, i'll take a look > Ah got it, so whether we are adding new or loading from the log we will a

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1797064629 > replay() is called when we read the records from the log Ah got it, so whether we are adding new or loading from the log we will always call replay 👍 -- This is an automated m

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1797063192 Also @jeffkbkim can you check the build? Seems like something is off. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384119244 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ## @@ -120,6 +121,9 @@ import static org.apache.kafka.coordinator.gro

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384118356 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -160,11 +161,14 @@ public GroupCoordinatorShard build() {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384116373 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache Sof

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384117312 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache Sof

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384116373 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache Sof

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1796859507 > I didn't know where to comment, but do we update the metrics when we load in a shard (ie, all the group states when we loaded etc) https://github.com/apache/kafka/pull/14387/fi

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384091707 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache S

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384085692 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache S

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384084418 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ## @@ -120,6 +121,9 @@ import static org.apache.kafka.coordinator.g

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384082333 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -160,11 +161,14 @@ public GroupCoordinatorShard build() {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1384079717 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ## @@ -472,7 +472,6 @@ public int deleteAllOffsets(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1793281394 I didn't know where to comment, but do we update the metrics when we load in a shard (ie, all the group states when we loaded etc) -- This is an automated message from the Apache Git Se

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382297632 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/CoordinatorMetricsShard.java: ## @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382297532 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/CoordinatorMetricsShard.java: ## @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382296446 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache Sof

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382296159 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorMetricsShard.java: ## @@ -0,0 +1,325 @@ +/* + * Licensed to the Apache Sof

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382292423 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java: ## @@ -120,6 +121,9 @@ import static org.apache.kafka.coordinator.gro

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1381915750 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: ooof. I'm too slow at reviewing :( -- This is an aut

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382183515 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -160,11 +161,14 @@ public GroupCoordinatorShard build() {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1382181160 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ## @@ -472,7 +472,6 @@ public int deleteAllOffsets( }

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1381915750 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: ooof. I'm too slow at reviewing :( -- This is an aut

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1381174247 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: it's ready -- This is an automated message from the

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1380569548 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ## @@ -352,7 +374,14 @@ public CoordinatorResult commitOffset(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1380140828 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ## @@ -352,7 +374,14 @@ public CoordinatorResult commitOffset(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1380080252 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: still working on it, will let you know once it's ready

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1380072080 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -121,6 +126,20 @@ public CoordinatorShardBuilder withTimer(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on PR #14387: URL: https://github.com/apache/kafka/pull/14387#issuecomment-1789827363 I took a first pass at the files today, and will take another look more closely at the metrics themselves tomorrow or so. -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379424990 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: Are we planning not to add any of the new sensor metrics

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379424990 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: Are we planning not to add any of the new sensor metrics

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jeffkbkim commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379419320 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: yeah, unfortunately we need this for the existing metri

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379418802 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: Or do we have this just to cover the existing generic met

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379411500 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/metrics/CoordinatorMetricsShard.java: ## @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379402554 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ## @@ -352,7 +374,14 @@ public CoordinatorResult commitOffset(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379398861 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -503,6 +547,7 @@ public void onLoaded(MetadataImage newImage) {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379398861 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -503,6 +547,7 @@ public void onLoaded(MetadataImage newImage) {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379393439 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -140,7 +159,12 @@ public GroupCoordinatorShard build() {

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379389203 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorShard.java: ## @@ -121,6 +126,20 @@ public CoordinatorShardBuilder withTimer(

Re: [PR] KAFKA-14519; [2/N] New coordinator metrics [kafka]

jolshan commented on code in PR #14387: URL: https://github.com/apache/kafka/pull/14387#discussion_r1379388116 ## checkstyle/import-control.xml: ## @@ -240,6 +240,7 @@ + Review Comment: I recall we removed yammer metrics from the last pr, do w