Skip to content

Commit 28b852d

Browse files
authored
Fix faulty assertion in AutoForceMergeManagerTests (opensearch-project#19024)
This commit removes a non-deterministic sleep and properly asserts that the AutoForceMergeManager completes quickly when interrupted. The previous test logic had a race condition where it checked the interrupt status of the thread, but that could return false if the thread was in the process of catching the InterruptException but had not yet reset the interrupt flag of the thread. The new logic is to set the delay to be the maximum of 60 seconds and assert that the test thread exits more quickly than that due to being interrupted. Signed-off-by: Andrew Ross <[email protected]>
1 parent 2f416d3 commit 28b852d

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

server/src/test/java/org/opensearch/index/autoforcemerge/AutoForceMergeManagerTests.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.junit.Before;
4949

5050
import java.io.IOException;
51+
import java.time.Duration;
5152
import java.util.Arrays;
5253
import java.util.Collections;
5354
import java.util.HashMap;
@@ -531,15 +532,20 @@ public void testForceMergeOperationOnDataNodeWithThreadInterruption() throws Int
531532
when(indicesService.spliterator()).thenReturn(indexServices.spliterator());
532533

533534
AutoForceMergeManager autoForceMergeManager = clusterSetupWithNode(
534-
getConfiguredClusterSettings(true, true, Collections.emptyMap()),
535+
getConfiguredClusterSettings(
536+
true,
537+
true,
538+
// Configure maximum delay because this test will interrupt it
539+
Map.of(ForceMergeManagerSettings.MERGE_DELAY_BETWEEN_SHARDS_FOR_AUTO_FORCE_MERGE.getKey(), TimeValue.timeValueSeconds(60))
540+
),
535541
dataNode1
536542
);
537543
autoForceMergeManager.start();
538544
Thread testThread = new Thread(() -> autoForceMergeManager.getTask().runInternal());
539545
testThread.start();
540-
Thread.sleep(1000L);
541546
testThread.interrupt();
542-
assertTrue(testThread.isInterrupted());
547+
assertTrue("Expected testThread to exit quickly after being interrupted", testThread.join(Duration.ofSeconds(10)));
548+
assertTrue("Expected the interrupt status to be set", testThread.isInterrupted());
543549
autoForceMergeManager.close();
544550
executorService.shutdown();
545551
}

0 commit comments

Comments
 (0)