Fix ThreadPoolMergeSchedulerTests testSchedulerCloseWaitsForRunningMerge

muted-tests.yml

@@ -281,9 +281,6 @@ tests:
 - class: org.elasticsearch.search.basic.SearchWithRandomDisconnectsIT
   method: testSearchWithRandomDisconnects
   issue: https://github.com/elastic/elasticsearch/issues/122707
-- class: org.elasticsearch.index.engine.ThreadPoolMergeSchedulerTests
-  method: testSchedulerCloseWaitsForRunningMerge
-  issue: https://github.com/elastic/elasticsearch/issues/125236
 - class: org.elasticsearch.packaging.test.DockerTests
   method: test020PluginsListWithNoPlugins
   issue: https://github.com/elastic/elasticsearch/issues/126232
@@ -473,9 +470,6 @@ tests:
 - class: org.elasticsearch.packaging.test.DockerTests
   method: test081SymlinksAreFollowedWithEnvironmentVariableFiles
   issue: https://github.com/elastic/elasticsearch/issues/128867
-- class: org.elasticsearch.index.engine.ThreadPoolMergeExecutorServiceDiskSpaceTests
-  method: testAvailableDiskSpaceMonitorWhenFileSystemStatErrors
-  issue: https://github.com/elastic/elasticsearch/issues/129149
 - class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
   method: test {lookup-join.EnrichLookupStatsBug ASYNC}
   issue: https://github.com/elastic/elasticsearch/issues/129228

server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java

@@ -324,10 +324,19 @@ public void testDiskSpaceMonitorStartsAsDisabled() throws Exception {
     }
 
     public void testAvailableDiskSpaceMonitorWhenFileSystemStatErrors() throws Exception {
-        aFileStore.usableSpace = randomLongBetween(1L, 100L);
-        aFileStore.totalSpace = randomLongBetween(1L, 100L);
-        bFileStore.usableSpace = randomLongBetween(1L, 100L);
-        bFileStore.totalSpace = randomLongBetween(1L, 100L);
+        long aUsableSpace;
+        long bUsableSpace;
+        do {
+            aFileStore.usableSpace = randomLongBetween(1L, 1000L);
+            aFileStore.totalSpace = randomLongBetween(1L, 1000L);
+            bFileStore.usableSpace = randomLongBetween(1L, 1000L);
+            bFileStore.totalSpace = randomLongBetween(1L, 1000L);
+            // the default 5% (same as flood stage level)
+            aUsableSpace = Math.max(aFileStore.usableSpace - aFileStore.totalSpace / 20, 0L);
+            bUsableSpace = Math.max(bFileStore.usableSpace - bFileStore.totalSpace / 20, 0L);
+        } while (aUsableSpace == bUsableSpace); // they must be different in order to distinguish the available disk space updates
+        long finalBUsableSpace = bUsableSpace;
+        long finalAUsableSpace = aUsableSpace;
         boolean aErrorsFirst = randomBoolean();
         if (aErrorsFirst) {
             // the "a" file system will error when collecting stats
@@ -355,18 +364,10 @@ public void testAvailableDiskSpaceMonitorWhenFileSystemStatErrors() throws Excep
                     assertThat(availableDiskSpaceUpdates.size(), is(1));
                     if (aErrorsFirst) {
                         // uses the stats from "b"
-                        assertThat(
-                            availableDiskSpaceUpdates.getLast().getBytes(),
-                            // the default 5% (same as flood stage level)
-                            is(Math.max(bFileStore.usableSpace - bFileStore.totalSpace / 20, 0L))
-                        );
+                        assertThat(availableDiskSpaceUpdates.getLast().getBytes(), is(finalBUsableSpace));
                     } else {
                         // uses the stats from "a"
-                        assertThat(
-                            availableDiskSpaceUpdates.getLast().getBytes(),
-                            // the default 5% (same as flood stage level)
-                            is(Math.max(aFileStore.usableSpace - aFileStore.totalSpace / 20, 0L))
-                        );
+                        assertThat(availableDiskSpaceUpdates.getLast().getBytes(), is(finalAUsableSpace));
                     }
                 }
             });
@@ -393,21 +394,14 @@ public void testAvailableDiskSpaceMonitorWhenFileSystemStatErrors() throws Excep
             }
             assertBusy(() -> {
                 synchronized (availableDiskSpaceUpdates) {
+                    // the updates are different values
                     assertThat(availableDiskSpaceUpdates.size(), is(3));
                     if (aErrorsFirst) {
                         // uses the stats from "a"
-                        assertThat(
-                            availableDiskSpaceUpdates.getLast().getBytes(),
-                            // the default 5% (same as flood stage level)
-                            is(Math.max(aFileStore.usableSpace - aFileStore.totalSpace / 20, 0L))
-                        );
+                        assertThat(availableDiskSpaceUpdates.getLast().getBytes(), is(finalAUsableSpace));
                     } else {
                         // uses the stats from "b"
-                        assertThat(
-                            availableDiskSpaceUpdates.getLast().getBytes(),
-                            // the default 5% (same as flood stage level)
-                            is(Math.max(bFileStore.usableSpace - bFileStore.totalSpace / 20, 0L))
-                        );
+                        assertThat(availableDiskSpaceUpdates.getLast().getBytes(), is(finalBUsableSpace));
                     }
                 }
             });

server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeSchedulerTests.java

@@ -612,11 +612,12 @@ public void testSchedulerCloseWaitsForRunningMerge() throws Exception {
                         fail(e);
                     }
                 });
+                // test expects that there definitely is a running merge before closing the merge scheduler
+                mergeRunningLatch.await();
+                // closes the merge scheduler
                 t.start();
                 try {
                     assertTrue(t.isAlive());
-                    // wait for the merge to actually run
-                    mergeRunningLatch.await();
                     // ensure the merge scheduler is effectively "closed"
                     assertBusy(() -> {
                         MergeSource mergeSource2 = mock(MergeSource.class);