You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "UladzislauBlok (via GitHub)" <gi...@apache.org> on 2025/06/08 15:10:50 UTC
[PR] [WIP] Fix race condition in RemoteIndexCacheTest [kafka]
UladzislauBlok opened a new pull request, #19927:
URL: https://github.com/apache/kafka/pull/19927
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2365672133
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread with prefix:[%s] is alive", threadName));
Review Comment:
Thanks for the review. It's fixed now
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-3300159397
Hey @UladzislauBlok, I'll take a look asap, could you please update it with trunk latest changes in the meantime? 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 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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-3039484578
@chia7712 @FrankYang0529 Hey Guys, sorry for the ping. This PR was waiting for sometime. Could please take a look?
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2365919363
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread leak detected: ", threadName));
Review Comment:
or `"Thread leak detected: " + threadName`
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2365919243
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread leak detected: ", threadName));
Review Comment:
It should be `String.format("Thread leak detected: %s", threadName)`, right?
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2366219963
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
Review Comment:
Would you mind updating the documentation to match these changes?
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2366231236
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
Review Comment:
done
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-3155098224
@brandboat Hello, and sorry for ping. You have started CI pipeline for this MR. Could you please review this one?
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-2975000098
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-3162318069
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok closed pull request #19927: KAFKA-19299: Fix race condition in RemoteIndexCacheTest
URL: https://github.com/apache/kafka/pull/19927
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2356554041
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread with prefix:[%s] is alive", threadName));
Review Comment:
nit: seems clear to refer to the leak in the error message. Would be nice to be consistent with the similar validation we have on the new test infra: https://github.com/apache/kafka/blob/9a32a71e76da604f3e84734737fdf5e3fe9823f6/test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/junit/ClusterTestExtensions.java#L184
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19927:
URL: https://github.com/apache/kafka/pull/19927#issuecomment-3043378006
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm merged PR #19927:
URL: https://github.com/apache/kafka/pull/19927
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2366087976
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread leak detected: ", threadName));
Review Comment:
Yeh... I don't know how I missed this, should be good now
--
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
Re: [PR] KAFKA-19299: Fix race condition in RemoteIndexCacheTest [kafka]
Posted by "UladzislauBlok (via GitHub)" <gi...@apache.org>.
UladzislauBlok commented on code in PR #19927:
URL: https://github.com/apache/kafka/pull/19927#discussion_r2365672133
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread with prefix:[%s] is alive", threadName));
Review Comment:
Thanks for review. It fixed now
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -171,12 +170,9 @@ public static MetadataSnapshot metadataSnapshotWith(final int nodes, final Map<S
*
* @throws AssertionError If any thread with the specified name prefix and daemon status is found and is alive.
*/
- public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) {
- List<Thread> threads = Thread.getAllStackTraces().keySet().stream()
- .filter(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName))
- .collect(Collectors.toList());
- int threadCount = threads.size();
- assertEquals(0, threadCount);
+ public static void assertNoLeakedThreadsWithNameAndDaemonStatus(String threadName, boolean isDaemon) throws InterruptedException {
+ waitForCondition(() -> Thread.getAllStackTraces().keySet().stream()
+ .noneMatch(t -> t.isDaemon() == isDaemon && t.isAlive() && t.getName().startsWith(threadName)), String.format("Thread with prefix:[%s] is alive", threadName));
Review Comment:
Thanks for review. It's fixed now
--
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