Skip to content

Commit 92b0387

Browse files
albertzaharovitsalbertzaharovits
authored
assertTrue for submitMergeTask (#130430) (#130527)
Previously, out of zealousness for testing efficiency, the mocked filesystems were reused across the test suite class. But this makes tests liable to interference wrt to filesystem stats calls. Moreover, if one test fails, it can trigger failures in other test methods. This PR recreates the mocked filesystems for every test method. Fixes #129296 #130205
1 parent 89c46be commit 92b0387

File tree

1 file changed

+25
-63
lines changed

1 file changed

+25
-63
lines changed

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

Lines changed: 25 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727
import org.elasticsearch.threadpool.TestThreadPool;
2828
import org.elasticsearch.threadpool.ThreadPool;
2929
import org.junit.After;
30-
import org.junit.AfterClass;
31-
import org.junit.BeforeClass;
30+
import org.junit.Before;
3231

3332
import java.io.IOException;
3433
import java.nio.file.FileStore;
@@ -59,8 +58,8 @@
5958

6059
public class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase {
6160

62-
private static TestMockFileStore aFileStore = new TestMockFileStore("mocka");
63-
private static TestMockFileStore bFileStore = new TestMockFileStore("mockb");
61+
private static TestMockFileStore aFileStore;
62+
private static TestMockFileStore bFileStore;
6463
private static String aPathPart;
6564
private static String bPathPart;
6665
private static int mergeExecutorThreadCount;
@@ -69,8 +68,10 @@ public class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase {
6968
private static NodeEnvironment nodeEnvironment;
7069
private static boolean setThreadPoolMergeSchedulerSetting;
7170

72-
@BeforeClass
73-
public static void installMockUsableSpaceFS() throws Exception {
71+
@Before
72+
public void setupTestEnv() throws Exception {
73+
aFileStore = new TestMockFileStore("mocka");
74+
bFileStore = new TestMockFileStore("mockb");
7475
FileSystem current = PathUtils.getDefaultFileSystem();
7576
aPathPart = "a-" + randomUUID();
7677
bPathPart = "b-" + randomUUID();
@@ -96,20 +97,21 @@ public static void installMockUsableSpaceFS() throws Exception {
9697
nodeEnvironment = new NodeEnvironment(settings, TestEnvironment.newEnvironment(settings));
9798
}
9899

99-
@AfterClass
100-
public static void removeMockUsableSpaceFS() {
100+
@After
101+
public void removeMockUsableSpaceFS() {
102+
if (setThreadPoolMergeSchedulerSetting) {
103+
assertWarnings(
104+
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
105+
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
106+
);
107+
}
101108
PathUtilsForTesting.teardown();
102109
aFileStore = null;
103110
bFileStore = null;
104111
testThreadPool.close();
105112
nodeEnvironment.close();
106113
}
107114

108-
@After
109-
public void cleanupThreadPool() {
110-
testThreadPool.scheduledTasks.clear();
111-
}
112-
113115
static class TestCapturingThreadPool extends TestThreadPool {
114116
final List<Tuple<TimeValue, Cancellable>> scheduledTasks = new ArrayList<>();
115117

@@ -319,8 +321,6 @@ public void testDiskSpaceMonitorStartsAsDisabled() throws Exception {
319321
)
320322
);
321323
}
322-
aFileStore.throwIoException = false;
323-
bFileStore.throwIoException = false;
324324
}
325325

326326
public void testAvailableDiskSpaceMonitorWhenFileSystemStatErrors() throws Exception {
@@ -406,8 +406,6 @@ public void testAvailableDiskSpaceMonitorWhenFileSystemStatErrors() throws Excep
406406
}
407407
});
408408
}
409-
aFileStore.throwIoException = false;
410-
bFileStore.throwIoException = false;
411409
}
412410

413411
public void testAvailableDiskSpaceMonitorSettingsUpdate() throws Exception {
@@ -516,12 +514,6 @@ public void testAvailableDiskSpaceMonitorSettingsUpdate() throws Exception {
516514
}
517515
}, 5, TimeUnit.SECONDS);
518516
}
519-
if (setThreadPoolMergeSchedulerSetting) {
520-
assertWarnings(
521-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
522-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
523-
);
524-
}
525517
}
526518

527519
public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
@@ -564,7 +556,7 @@ public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
564556
testDoneLatch.await();
565557
return null;
566558
}).when(stallingMergeTask).abort();
567-
threadPoolMergeExecutorService.submitMergeTask(stallingMergeTask);
559+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(stallingMergeTask));
568560
// assert the merge task is holding up disk space budget
569561
expectedAvailableBudget.set(expectedAvailableBudget.get() - taskBudget);
570562
assertBusy(
@@ -574,7 +566,7 @@ public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
574566
ThreadPoolMergeScheduler.MergeTask mergeTask = mock(ThreadPoolMergeScheduler.MergeTask.class);
575567
when(mergeTask.estimatedRemainingMergeSize()).thenReturn(randomLongBetween(0L, expectedAvailableBudget.get()));
576568
when(mergeTask.schedule()).thenReturn(RUN);
577-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
569+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
578570
assertBusy(() -> {
579571
verify(mergeTask).schedule();
580572
verify(mergeTask).run();
@@ -595,12 +587,6 @@ public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
595587
assertThat(threadPoolMergeExecutorService.allDone(), is(true));
596588
});
597589
}
598-
if (setThreadPoolMergeSchedulerSetting) {
599-
assertWarnings(
600-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
601-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
602-
);
603-
}
604590
}
605591

606592
public void testBackloggedMergeTasksDoNotHoldUpBudget() throws Exception {
@@ -654,7 +640,7 @@ && randomBoolean()) {
654640
testDoneLatch.await();
655641
return null;
656642
}).when(mergeTask).abort();
657-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
643+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
658644
if (mergeTask.schedule() == RUN) {
659645
runningMergeTasks.add(mergeTask);
660646
} else {
@@ -679,7 +665,7 @@ && randomBoolean()) {
679665
return RUN;
680666
}
681667
}).when(mergeTask).schedule();
682-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
668+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
683669
backloggingMergeTasksScheduleCountMap.put(mergeTask, 1);
684670
}
685671
int checkRounds = randomIntBetween(1, 10);
@@ -712,7 +698,7 @@ && randomBoolean()) {
712698
long taskBudget = randomLongBetween(1L, backloggedMergeTaskDiskSpaceBudget);
713699
when(mergeTask.estimatedRemainingMergeSize()).thenReturn(taskBudget);
714700
when(mergeTask.schedule()).thenReturn(RUN);
715-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
701+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
716702
assertBusy(() -> {
717703
verify(mergeTask).schedule();
718704
verify(mergeTask).run();
@@ -739,12 +725,6 @@ && randomBoolean()) {
739725
assertThat(threadPoolMergeExecutorService.allDone(), is(true));
740726
});
741727
}
742-
if (setThreadPoolMergeSchedulerSetting) {
743-
assertWarnings(
744-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
745-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
746-
);
747-
}
748728
}
749729

750730
public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() throws Exception {
@@ -823,7 +803,7 @@ public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() thro
823803
runningOrAbortingMergeTasksList.add(mergeTask);
824804
latchesBlockingMergeTasksList.add(blockMergeTaskLatch);
825805
}
826-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
806+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
827807
}
828808
// currently running (or aborting) merge tasks have consumed some of the available budget
829809
while (runningOrAbortingMergeTasksList.isEmpty() == false) {
@@ -855,8 +835,8 @@ public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() thro
855835
// merge task 2 can run because it is under budget
856836
when(mergeTask2.estimatedRemainingMergeSize()).thenReturn(underBudget);
857837
}
858-
threadPoolMergeExecutorService.submitMergeTask(mergeTask1);
859-
threadPoolMergeExecutorService.submitMergeTask(mergeTask2);
838+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask1));
839+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask2));
860840
assertBusy(() -> {
861841
if (task1Runs) {
862842
verify(mergeTask1).schedule();
@@ -890,12 +870,6 @@ public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() thro
890870
bFileStore.usableSpace = Long.MAX_VALUE;
891871
assertBusy(() -> assertThat(threadPoolMergeExecutorService.allDone(), is(true)));
892872
}
893-
if (setThreadPoolMergeSchedulerSetting) {
894-
assertWarnings(
895-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
896-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
897-
);
898-
}
899873
}
900874

901875
public void testEnqueuedMergeTasksAreUnblockedWhenEstimatedMergeSizeChanges() throws Exception {
@@ -990,12 +964,6 @@ public void testEnqueuedMergeTasksAreUnblockedWhenEstimatedMergeSizeChanges() th
990964
}
991965
});
992966
}
993-
if (setThreadPoolMergeSchedulerSetting) {
994-
assertWarnings(
995-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
996-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
997-
);
998-
}
999967
}
1000968

1001969
public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws Exception {
@@ -1058,7 +1026,7 @@ public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws
10581026
testDoneLatch.await();
10591027
return null;
10601028
}).when(mergeTask).abort();
1061-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
1029+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
10621030
if (mergeTask.schedule() == RUN) {
10631031
runningMergeTasks.add(mergeTask);
10641032
} else {
@@ -1083,7 +1051,7 @@ public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws
10831051
when(mergeTask.estimatedRemainingMergeSize()).thenReturn(taskBudget);
10841052
Schedule schedule = randomFrom(RUN, ABORT);
10851053
when(mergeTask.schedule()).thenReturn(schedule);
1086-
threadPoolMergeExecutorService.submitMergeTask(mergeTask);
1054+
assertTrue(threadPoolMergeExecutorService.submitMergeTask(mergeTask));
10871055
if (schedule == RUN) {
10881056
overBudgetTasksToRunList.add(mergeTask);
10891057
} else {
@@ -1150,11 +1118,5 @@ public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws
11501118
);
11511119
});
11521120
}
1153-
if (setThreadPoolMergeSchedulerSetting) {
1154-
assertWarnings(
1155-
"[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
1156-
+ "and will be removed in a future release. See the breaking changes documentation for the next major version."
1157-
);
1158-
}
11591121
}
11601122
}

0 commit comments

Comments
 (0)