NodeJoinTests fix for future complete assertion

NodeJoinTests.testConcurrentJoining use their own threads, but the thread name parsing done for validating that wait and complete of future is on different executors would pick up more or less random fields. Now disregard these test threads for that assertion. Closes elastic#109103
henningandersen · May 28, 2024 · aadbe18 · aadbe18
1 parent 6d86415
commit aadbe18
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 4 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java
@@ -270,7 +270,7 @@ public static String executorName(String threadName) {
         // subtract 2 to avoid the `]` of the thread number part.
         int executorNameEnd = threadName.lastIndexOf(']', threadName.length() - 2);
         int executorNameStart = threadName.lastIndexOf('[', executorNameEnd);
-        if (executorNameStart == -1 || executorNameEnd - executorNameStart <= 1) {
+        if (executorNameStart == -1 || executorNameEnd - executorNameStart <= 1 || threadName.startsWith("TEST-")) {
             return null;
         }
         return threadName.substring(executorNameStart + 1, executorNameEnd);

diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/NodeJoinTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/NodeJoinTests.java
@@ -678,7 +678,6 @@ public void testBecomeFollowerFailsPendingJoin() throws Exception {
         assertFalse(isLocalNodeElectedMaster());
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/109103")
     public void testConcurrentJoining() {
         List<DiscoveryNode> masterNodes = IntStream.rangeClosed(1, randomIntBetween(2, 5))
             .mapToObj(nodeId -> newNode(nodeId, true))
@@ -775,14 +774,14 @@ public void testConcurrentJoining() {
         final List<Thread> joinThreads = Stream.concat(correctJoinRequests.stream().map(joinRequest -> new Thread(() -> {
             safeAwait(barrier);
             joinNode(joinRequest);
-        }, "process " + joinRequest)), possiblyFailingJoinRequests.stream().map(joinRequest -> new Thread(() -> {
+        }, "TEST-process " + joinRequest)), possiblyFailingJoinRequests.stream().map(joinRequest -> new Thread(() -> {
             safeAwait(barrier);
             try {
                 joinNode(joinRequest);
             } catch (CoordinationStateRejectedException e) {
                 // ignore - these requests are expected to fail
             }
-        }, "process " + joinRequest))).toList();
+        }, "TEST-process " + joinRequest))).toList();
 
         assertionThread.start();
         joinThreads.forEach(Thread::start);

diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java
@@ -37,6 +37,7 @@
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThan;
+import static org.hamcrest.Matchers.nullValue;
 
 /**
  * Tests for EsExecutors and its components like EsAbortPolicy.
@@ -646,6 +647,8 @@ public void testParseExecutorName() throws InterruptedException {
         final var thread = threadFactory.newThread(() -> {});
         try {
             assertThat(EsExecutors.executorName(thread.getName()), equalTo(executorName));
+            assertThat(EsExecutors.executorName(thread), equalTo(executorName));
+            assertThat(EsExecutors.executorName("TEST-" + thread.getName()), is(nullValue()));
         } finally {
             thread.join();
         }