From 46206e1be39c9356347a7a80f3cebc8560205892 Mon Sep 17 00:00:00 2001 From: panguixin Date: Tue, 12 Dec 2023 03:26:53 +0800 Subject: [PATCH 1/5] fix fs info reporting negative available size Signed-off-by: panguixin --- .../org/opensearch/monitor/fs/FsProbe.java | 4 ++ .../opensearch/monitor/fs/FsProbeTests.java | 57 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java b/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java index f4731a4a34373..f93cb63ff1f0a 100644 --- a/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java +++ b/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java @@ -82,6 +82,10 @@ public FsInfo stats(FsInfo previous) throws IOException { paths[i].fileCacheReserved = adjustForHugeFilesystems(dataLocations[i].fileCacheReservedSize.getBytes()); paths[i].fileCacheUtilized = adjustForHugeFilesystems(fileCache.usage().usage()); paths[i].available -= (paths[i].fileCacheReserved - paths[i].fileCacheUtilized); + // occurs if reserved file cache space is occupied by other files, like local indices + if (paths[i].available < 0) { + paths[i].available = 0; + } } } FsInfo.IoStats ioStats = null; diff --git a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java index 59a888c665be7..d4b58847f4650 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -45,9 +45,12 @@ import org.opensearch.index.store.remote.filecache.FileCacheFactory; import org.opensearch.test.OpenSearchTestCase; +import java.io.BufferedWriter; import java.io.IOException; import java.nio.file.FileStore; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.FileStoreAttributeView; import java.util.Arrays; @@ -58,6 +61,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import static org.opensearch.monitor.fs.FsProbe.adjustForHugeFilesystems; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.emptyOrNullString; import static org.hamcrest.Matchers.greaterThan; @@ -162,6 +166,59 @@ public void testFsCacheInfo() throws IOException { } } + public void testFsInfoWhenFileCacheOccupied() throws IOException { + Settings settings = Settings.builder().putList("node.roles", "search", "data").build(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + final long usableSpace = adjustForHugeFilesystems(env.fileCacheNodePath().fileStore.getUsableSpace()); + ByteSizeValue gbByteSizeValue = new ByteSizeValue(usableSpace - 10, ByteSizeUnit.BYTES); + env.fileCacheNodePath().fileCacheReservedSize = gbByteSizeValue; + FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( + gbByteSizeValue.getBytes(), + 16, + new NoopCircuitBreaker(CircuitBreaker.REQUEST) + ); + + // write a temp file to occupy some file cache space + Path tempFile = createTempFile(); + try (BufferedWriter bufferedWriter = Files.newBufferedWriter(tempFile, StandardOpenOption.APPEND)) { + bufferedWriter.write(randomAlphaOfLength(100)); + bufferedWriter.flush(); + } + + FsProbe probe = new FsProbe(env, fileCache); + FsInfo stats = probe.stats(null); + assertNotNull(stats); + assertTrue(stats.getTimestamp() > 0L); + FsInfo.Path total = stats.getTotal(); + assertNotNull(total); + assertTrue(total.total > 0L); + assertTrue(total.free > 0L); + assertTrue(total.fileCacheReserved > 0L); + if (env.nodePaths().length > 1) { + assertTrue(total.available > 0L); + } else { + assertEquals(0L, total.available); + } + assertTrue((total.free - total.available) < total.fileCacheReserved); + + for (FsInfo.Path path : stats) { + assertNotNull(path); + assertFalse(path.getPath().isEmpty()); + assertFalse(path.getMount().isEmpty()); + assertFalse(path.getType().isEmpty()); + assertTrue(path.total > 0L); + assertTrue(path.free > 0L); + + if (path.fileCacheReserved > 0L) { + assertEquals(0L, path.available); + assertTrue(path.free - path.available < path.fileCacheReserved); + } else { + assertTrue(path.available > 0L); + } + } + } + } + public void testFsInfoOverflow() throws Exception { final FsInfo.Path pathStats = new FsInfo.Path( "/foo/bar", From 6d838b417064300e693a20aac594cfa31478d81a Mon Sep 17 00:00:00 2001 From: panguixin Date: Tue, 12 Dec 2023 20:21:11 +0800 Subject: [PATCH 2/5] change log Signed-off-by: panguixin --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0266d2a6dd4e..8a625bc697c26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed rest-high-level client searchTemplate & mtermVectors endpoints to have a leading slash ([#14465](https://github.com/opensearch-project/OpenSearch/pull/14465)) - Write shard level metadata blob when snapshotting searchable snapshot indexes ([#13190](https://github.com/opensearch-project/OpenSearch/pull/13190)) - Fix aggs result of NestedAggregator with sub NestedAggregator ([#13324](https://github.com/opensearch-project/OpenSearch/pull/13324)) +- Fix fs info reporting negative available size ([#11573](https://github.com/opensearch-project/OpenSearch/pull/11573)) ### Security From 52296bf1094668a929f09499bd7d60f8dce5e6f7 Mon Sep 17 00:00:00 2001 From: panguixin Date: Wed, 13 Dec 2023 20:10:36 +0800 Subject: [PATCH 3/5] fix test Signed-off-by: panguixin --- .../src/test/java/org/opensearch/monitor/fs/FsProbeTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java index d4b58847f4650..26e5f60e9c91e 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -199,7 +199,6 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { } else { assertEquals(0L, total.available); } - assertTrue((total.free - total.available) < total.fileCacheReserved); for (FsInfo.Path path : stats) { assertNotNull(path); @@ -211,7 +210,6 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { if (path.fileCacheReserved > 0L) { assertEquals(0L, path.available); - assertTrue(path.free - path.available < path.fileCacheReserved); } else { assertTrue(path.available > 0L); } From 902cbbee09450b74a48d781303e95778de5c7621 Mon Sep 17 00:00:00 2001 From: panguixin Date: Fri, 15 Dec 2023 20:00:28 +0800 Subject: [PATCH 4/5] fix test Signed-off-by: panguixin --- .../org/opensearch/monitor/fs/FsProbeTests.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java index 26e5f60e9c91e..95664f63c3c90 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -169,8 +169,9 @@ public void testFsCacheInfo() throws IOException { public void testFsInfoWhenFileCacheOccupied() throws IOException { Settings settings = Settings.builder().putList("node.roles", "search", "data").build(); try (NodeEnvironment env = newNodeEnvironment(settings)) { - final long usableSpace = adjustForHugeFilesystems(env.fileCacheNodePath().fileStore.getUsableSpace()); - ByteSizeValue gbByteSizeValue = new ByteSizeValue(usableSpace - 10, ByteSizeUnit.BYTES); + // Use the total space as reserved space to simulate the situation where the cache space is occupied + final long totalSpace = adjustForHugeFilesystems(env.fileCacheNodePath().fileStore.getTotalSpace()); + ByteSizeValue gbByteSizeValue = new ByteSizeValue(totalSpace, ByteSizeUnit.BYTES); env.fileCacheNodePath().fileCacheReservedSize = gbByteSizeValue; FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( gbByteSizeValue.getBytes(), @@ -178,13 +179,6 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { new NoopCircuitBreaker(CircuitBreaker.REQUEST) ); - // write a temp file to occupy some file cache space - Path tempFile = createTempFile(); - try (BufferedWriter bufferedWriter = Files.newBufferedWriter(tempFile, StandardOpenOption.APPEND)) { - bufferedWriter.write(randomAlphaOfLength(100)); - bufferedWriter.flush(); - } - FsProbe probe = new FsProbe(env, fileCache); FsInfo stats = probe.stats(null); assertNotNull(stats); @@ -194,11 +188,6 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { assertTrue(total.total > 0L); assertTrue(total.free > 0L); assertTrue(total.fileCacheReserved > 0L); - if (env.nodePaths().length > 1) { - assertTrue(total.available > 0L); - } else { - assertEquals(0L, total.available); - } for (FsInfo.Path path : stats) { assertNotNull(path); From 1bae5b8fe6b16267c3f9b9877ed02dd4ee406381 Mon Sep 17 00:00:00 2001 From: panguixin Date: Tue, 6 Feb 2024 22:31:42 +0800 Subject: [PATCH 5/5] spotless Signed-off-by: panguixin --- .../src/test/java/org/opensearch/monitor/fs/FsProbeTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java index 95664f63c3c90..e2e09d5ce63fe 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -45,12 +45,9 @@ import org.opensearch.index.store.remote.filecache.FileCacheFactory; import org.opensearch.test.OpenSearchTestCase; -import java.io.BufferedWriter; import java.io.IOException; import java.nio.file.FileStore; -import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.FileStoreAttributeView; import java.util.Arrays;