From a6004a6067f4a0aee0c582b095f3547f78cd6f85 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 26 Jun 2025 14:05:40 +0300 Subject: [PATCH] Fix ThreadPoolMergeExecutorServiceDiskSpaceTests testAvailableDiskSpaceMonitorWhenFileSystemStatErrors (#130025) The test method needs to distinguish between the available disk space update values, when they are coming from either FS. So the update values from the 2 FSs mustn't be equal. Fixes #129149 --- muted-tests.yml | 3 -- ...oolMergeExecutorServiceDiskSpaceTests.java | 42 ++++++++----------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 6c00790d94a2..6fe65c95ea10 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -473,9 +473,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 diff --git a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java b/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java index 4653f8210cda..8a497d09320f 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java @@ -324,10 +324,19 @@ public class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase { } 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 class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase { 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 class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase { } 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)); } } });