Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437350376 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437350213 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2639,59 +2639,65 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
satishd commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437344252 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -167,7 +167,7 @@ class ReplicaManagerTest { .foreach(checkpointFile => assertTrue(File

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
satishd commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437341409 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
jolshan commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437157116 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2639,59 +2639,65 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
divijvaidya commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437034332 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437026129 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437024347 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time.sch

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
divijvaidya commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1437022630 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2704,28 +2710,30 @@ class ReplicaManagerTest { time = time, scheduler = time

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on PR #15077: URL: https://github.com/apache/kafka/pull/15077#issuecomment-1870263212 @satishd , I've closed all the instances in finally block quietly in this commit: https://github.com/apache/kafka/pull/15077/commits/dd913a8668cf773a51403e482cc704b44cb0e8a1 . Thanks.

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
satishd commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1436993064 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -4020,6 +4031,7 @@ class ReplicaManagerTest { doneLatch.countDown() } finally {

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1436972102 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2693,6 +2693,9 @@ class ReplicaManagerTest { } else { assertTrue(stray0.isInstance

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
divijvaidya commented on code in PR #15077: URL: https://github.com/apache/kafka/pull/15077#discussion_r1436885274 ## core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala: ## @@ -2693,6 +2693,9 @@ class ReplicaManagerTest { } else { assertTrue(stray0.isInst

Re: [PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon commented on PR #15077: URL: https://github.com/apache/kafka/pull/15077#issuecomment-1870075510 @dajac @jolshan @satishd , call for review. Thanks. -- 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

[PR] MINOR: close leaking threads in replica manager tests [kafka]

2023-12-27 Thread via GitHub
showuon opened a new pull request, #15077: URL: https://github.com/apache/kafka/pull/15077 Following @dajac 's finding in https://github.com/apache/kafka/pull/15063, I found we also create new RemoteLogManager in ReplicaManagerTest, but didn't close them. While investigating Replica