You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/12/01 20:22:57 UTC

[GitHub] [kafka] mumrah opened a new pull request, #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

mumrah opened a new pull request, #12942:
URL: https://github.com/apache/kafka/pull/12942

   The anonymous functions we create in classes like Partition will end up getting held by the singleton Yammer metrics registry. 
   
   ```
     newGauge("UnderReplicated", () => if (isUnderReplicated) 1 else 0, tags)
     newGauge("InSyncReplicasCount", () => if (isLeader) partitionState.isr.size else 0, tags)
     newGauge("UnderMinIsr", () => if (isUnderMinIsr) 1 else 0, tags)
     newGauge("AtMinIsr", () => if (isAtMinIsr) 1 else 0, tags)
     newGauge("ReplicasCount", () => if (isLeader) assignmentState.replicationFactor else 0, tags)
     newGauge("LastStableOffsetLag", () => log.map(_.lastStableOffsetLag).getOrElse(0), tags)
   ```
   
   This prevents GC from happening during test runs where we create lots of new BrokerServer/ControllerServer objects. In particular, we were seeing leaks of KafkaRaftClient which retains a fair amount of heap.
   
   This patch is a quick fix is to remove all the Yammer metrics from the registry between each test run. This should allow GC to occur as expected and improve overall heap usage during a test run.


-- 
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] mumrah merged pull request #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

Posted by GitBox <gi...@apache.org>.
mumrah merged PR #12942:
URL: https://github.com/apache/kafka/pull/12942


-- 
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] hachikuji commented on a diff in pull request #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12942:
URL: https://github.com/apache/kafka/pull/12942#discussion_r1037543086


##########
core/src/test/scala/integration/kafka/server/QuorumTestHarness.scala:
##########
@@ -386,6 +386,7 @@ abstract class QuorumTestHarness extends Logging {
   def tearDown(): Unit = {
     Exit.resetExitProcedure()
     Exit.resetHaltProcedure()
+    TestUtils.clearYammerMetrics()
     if (implementation != null) {
       implementation.shutdown()
     }

Review Comment:
   ```suggestion
       if (implementation != null) {
         implementation.shutdown()
       }
       TestUtils.clearYammerMetrics()
   ```
   



-- 
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] mumrah commented on pull request #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

Posted by GitBox <gi...@apache.org>.
mumrah commented on PR #12942:
URL: https://github.com/apache/kafka/pull/12942#issuecomment-1334405072

   Prior to this patch, I was seeing several KafkaRaftClient (and KafkaRaftManager) objects hanging around in memory after running the test suite for 30 minutes or so. 
   
   <img width="803" alt="image" src="https://user-images.githubusercontent.com/55116/205152421-fe754e5c-a6de-4b5d-8184-99243c3a5955.png">
   
   With this patch, I only see 1 as expected:
   
   <img width="799" alt="image" src="https://user-images.githubusercontent.com/55116/205152320-2448db3f-e001-4319-b94a-f1abc349b1cb.png">
   
   


-- 
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] hachikuji commented on a diff in pull request #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12942:
URL: https://github.com/apache/kafka/pull/12942#discussion_r1037533828


##########
core/src/test/scala/integration/kafka/server/QuorumTestHarness.scala:
##########
@@ -386,6 +386,7 @@ abstract class QuorumTestHarness extends Logging {
   def tearDown(): Unit = {
     Exit.resetExitProcedure()
     Exit.resetHaltProcedure()
+    TestUtils.clearYammerMetrics()

Review Comment:
   Actually, should we do this after the implementation has shutdown? I don't know if the metrics would be used or recreated during shutdown.



-- 
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] hachikuji commented on a diff in pull request #12942: KAFKA-14433 Clear Yammer metrics in QuorumTestHarness#tearDown

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12942:
URL: https://github.com/apache/kafka/pull/12942#discussion_r1037533828


##########
core/src/test/scala/integration/kafka/server/QuorumTestHarness.scala:
##########
@@ -386,6 +386,7 @@ abstract class QuorumTestHarness extends Logging {
   def tearDown(): Unit = {
     Exit.resetExitProcedure()
     Exit.resetHaltProcedure()
+    TestUtils.clearYammerMetrics()

Review Comment:
   Actually, should we do this after the implementation has shutdown? I don't know if the metrics would be used during shutdown



-- 
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