diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md index 06b761b1df8bd..f4166cf3e6623 100644 --- a/CHANGELOG-3.0.md +++ b/CHANGELOG-3.0.md @@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827)) - Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944)) - Don't over-allocate in HeapBufferedAsyncEntityConsumer in order to consume the response ([#9993](https://github.com/opensearch-project/OpenSearch/pull/9993)) +- Display all file cache metrics in NodeStats and use the correct human-readable field name ([#13232](https://github.com/opensearch-project/OpenSearch/pull/13232)) ### Security diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/70_file_cache.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/70_file_cache.yml new file mode 100644 index 0000000000000..9ed45d7a4f396 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.stats/70_file_cache.yml @@ -0,0 +1,35 @@ +# As of now only nodes with the "search" role can utilize file caching. +# Moreover, until the remote store is used in the test and some searches executed (which will force +# downloading the files from remote store to node file cache) we should always expect 0b size. +# But at least the cache file fields must be present. +--- +"File Cache stats": + - skip: + version: " - 2.15.99" + reason: "file cache statistics fields were added in 2.7 (#6485) but the fix (#13232) was merged in 2.16.0" + features: [arbitrary_key, node_selector] + + - do: + node_selector: + version: "3.0.0 - " + nodes.info: {} + - set: + nodes._arbitrary_key_: node_id + + - do: + node_selector: + version: "3.0.0 - " + nodes.stats: + metric: [ fs ] + + # In the future we shall test that the node has a "search" role + # otherwise the file cache will be always 0. + - is_true: nodes.$node_id.roles + + - is_true: nodes.$node_id.fs.total + - gte: { nodes.$node_id.fs.total.cache_reserved_in_bytes: 0 } + - gte: { nodes.$node_id.fs.total.cache_utilized_in_bytes: 0 } + + - is_true: nodes.$node_id.fs.data + - gte: { nodes.$node_id.fs.data.0.cache_reserved_in_bytes: 0 } + - gte: { nodes.$node_id.fs.data.0.cache_utilized_in_bytes: 0 } diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java index cc8747e5f5666..b3142e358b0d1 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderIT.java @@ -563,7 +563,7 @@ private long createReasonableSizedShards(final String indexName) throws Interrup } private static FsInfo.Path setDiskUsage(FsInfo.Path original, long totalBytes, long freeBytes) { - return new FsInfo.Path(original.getPath(), original.getMount(), totalBytes, freeBytes, freeBytes); + return new FsInfo.Path(original.getPath(), original.getMount(), totalBytes, freeBytes, freeBytes, 0, 0); } private void refreshDiskUsage() { diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/MockDiskUsagesIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/MockDiskUsagesIT.java index a7755841c9ea9..718c06820bc9b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/MockDiskUsagesIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/routing/allocation/decider/MockDiskUsagesIT.java @@ -89,7 +89,7 @@ public Settings indexSettings() { } private static FsInfo.Path setDiskUsage(FsInfo.Path original, long totalBytes, long freeBytes) { - return new FsInfo.Path(original.getPath(), original.getMount(), totalBytes, freeBytes, freeBytes); + return new FsInfo.Path(original.getPath(), original.getMount(), totalBytes, freeBytes, freeBytes, 0, 0); } public void testRerouteOccursOnDiskPassingHighWatermark() throws Exception { diff --git a/server/src/main/java/org/opensearch/env/NodeEnvironment.java b/server/src/main/java/org/opensearch/env/NodeEnvironment.java index 2748938d8b761..b127400ba4712 100644 --- a/server/src/main/java/org/opensearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/opensearch/env/NodeEnvironment.java @@ -71,6 +71,7 @@ import org.opensearch.index.IndexSettings; import org.opensearch.index.shard.ShardPath; import org.opensearch.index.store.FsDirectoryFactory; +import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.monitor.fs.FsInfo; import org.opensearch.monitor.fs.FsProbe; import org.opensearch.monitor.jvm.JvmInfo; @@ -126,6 +127,7 @@ public static class NodePath { public final Path indicesPath; /** Cached FileStore from path */ public final FileStore fileStore; + /* ${data.paths}/nodes/{node.id}/cache */ public final Path fileCachePath; /* Cache reserved size can default to a different value depending on configuration @@ -178,6 +180,8 @@ public String toString() { + indicesPath + ", fileStore=" + fileStore + + ", fileCachePath=" + + fileCachePath + ", majorDeviceNumber=" + majorDeviceNumber + ", minorDeviceNumber=" @@ -421,7 +425,7 @@ private void maybeLogPathDetails() throws IOException { for (NodePath nodePath : nodePaths) { sb.append('\n').append(" -> ").append(nodePath.path.toAbsolutePath()); - FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath); + FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath, FileCache.NOOP_FILE_CACHE); sb.append(", free_space [") .append(fsPath.getFree()) .append("], usable_space [") @@ -440,7 +444,7 @@ private void maybeLogPathDetails() throws IOException { Set allTypes = new HashSet<>(); Set allMounts = new HashSet<>(); for (NodePath nodePath : nodePaths) { - FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath); + FsInfo.Path fsPath = FsProbe.getFSInfo(nodePath, FileCache.NOOP_FILE_CACHE); String mount = fsPath.getMount(); if (allMounts.contains(mount) == false) { allMounts.add(mount); diff --git a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java index a8aa33a977cb8..2ca39e09c4518 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java +++ b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java @@ -14,6 +14,8 @@ import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.breaker.CircuitBreaker; import org.opensearch.core.common.breaker.CircuitBreakingException; +import org.opensearch.core.common.breaker.NoopCircuitBreaker; +import org.opensearch.env.NodeEnvironment; import org.opensearch.index.store.remote.utils.cache.CacheUsage; import org.opensearch.index.store.remote.utils.cache.RefCountedCache; import org.opensearch.index.store.remote.utils.cache.SegmentedCache; @@ -49,6 +51,29 @@ */ @PublicApi(since = "2.7.0") public class FileCache implements RefCountedCache { + + /** + * A minimalistic instance of the File Cache (FC) which is not useful for any real caching task + * but is useful as a placeholder in API calls that require FC instance, for example + * {@link org.opensearch.monitor.fs.FsProbe#getFSInfo(NodeEnvironment.NodePath, FileCache)}. + *
+ * REMEMBER to use a valid FC instance if the output of such API call needs to reflect its state. + * + * @opensearch.internal + */ + public static FileCache NOOP_FILE_CACHE = null; + + // Perhaps we shall create a new class with static instance like NoopFileCache.INSTANCE ??? + static { + int CONCURRENCY_LEVEL = 1; + int CAPACITY = 0; + NOOP_FILE_CACHE = FileCacheFactory.createConcurrentLRUFileCache( + CAPACITY, + CONCURRENCY_LEVEL, + new NoopCircuitBreaker(CircuitBreaker.REQUEST) + ); + } + private static final Logger logger = LogManager.getLogger(FileCache.class); private final SegmentedCache theCache; diff --git a/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java b/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java index ddff022112665..67a2f6c43fc84 100644 --- a/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java +++ b/server/src/main/java/org/opensearch/monitor/fs/FsInfo.java @@ -34,6 +34,7 @@ import org.opensearch.Version; import org.opensearch.common.Nullable; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -75,16 +76,48 @@ public static class Path implements Writeable, ToXContentObject { long free = -1; long available = -1; long fileCacheReserved = -1; - long fileCacheUtilized = 0; + long fileCacheUtilized = -1; public Path() {} + /** + * Please notice that this constructor will set value of 0 + * to fileCacheReserved and fileCacheUtilized variables. + * + * See {@link #getFileCacheReserved()}, {@link #getFileCacheUtilized()} + */ public Path(String path, @Nullable String mount, long total, long free, long available) { + new Path(path, mount, total, free, available, 0, 0); + } + + /** + * Do not assign negative values to variables total, free, available, + * fileCacheReserved and fileCacheUtilized. The only exception is value -1 + * which is interpreted as an uninitialized value. + */ + @InternalApi + public Path( + String path, + @Nullable String mount, + long total, + long free, + long available, + long fileCacheReserved, + long fileCacheUtilized + ) { this.path = path; this.mount = mount; this.total = total; this.free = free; this.available = available; + this.fileCacheReserved = fileCacheReserved; + this.fileCacheUtilized = fileCacheUtilized; + + assert (this.total == -1 || this.total >= 0) : "Total value can not be negative"; + assert (this.free == -1 || this.free >= 0) : "Free value can not be negative"; + assert (this.available == -1 || this.available >= 0) : "Available value can not be negative"; + assert (this.fileCacheReserved == -1 || this.fileCacheReserved >= 0) : "File cache reserved value can not be negative"; + assert (this.fileCacheUtilized == -1 || this.fileCacheUtilized >= 0) : "File cache utilization value can not be negative"; } /** @@ -211,8 +244,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (fileCacheReserved != -1) { builder.humanReadableField(Fields.CACHE_RESERVED_IN_BYTES, Fields.CACHE_RESERVED, getFileCacheReserved()); } - if (fileCacheReserved != 0) { - builder.humanReadableField(Fields.CACHE_UTILIZED, Fields.CACHE_UTILIZED_IN_BYTES, getFileCacheUtilized()); + if (fileCacheUtilized != -1) { + builder.humanReadableField(Fields.CACHE_UTILIZED_IN_BYTES, Fields.CACHE_UTILIZED, getFileCacheUtilized()); } builder.endObject(); 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 db77ec7628e76..d17fa9fbec77e 100644 --- a/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java +++ b/server/src/main/java/org/opensearch/monitor/fs/FsProbe.java @@ -37,6 +37,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.util.Constants; import org.opensearch.common.SuppressForbidden; +import org.opensearch.common.annotation.DeprecatedApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.io.PathUtils; import org.opensearch.core.common.unit.ByteSizeValue; @@ -77,8 +78,8 @@ public FsInfo stats(FsInfo previous) throws IOException { NodePath[] dataLocations = nodeEnv.nodePaths(); FsInfo.Path[] paths = new FsInfo.Path[dataLocations.length]; for (int i = 0; i < dataLocations.length; i++) { - paths[i] = getFSInfo(dataLocations[i]); - if (fileCache != null && dataLocations[i].fileCacheReservedSize != ByteSizeValue.ZERO) { + paths[i] = getFSInfo(dataLocations[i], fileCache == null ? FileCache.NOOP_FILE_CACHE : fileCache); + if (fileCache != null && dataLocations[i].fileCacheReservedSize.compareTo(ByteSizeValue.ZERO) >= 0) { paths[i].fileCacheReserved = adjustForHugeFilesystems(dataLocations[i].fileCacheReservedSize.getBytes()); paths[i].fileCacheUtilized = adjustForHugeFilesystems(fileCache.usage().usage()); // fileCacheFree will be less than zero if the cache being over-subscribed @@ -203,6 +204,14 @@ static long adjustForHugeFilesystems(long bytes) { return bytes; } + /** + * Retrieves information about the path associated with the given node path. + * + * @deprecated This method has been deprecated since version 2.16.0 and will be removed in version 3.0.0, use {@link #getFSInfo(NodePath, FileCache)} instead. + * @see #getFSInfo(NodePath, FileCache) + */ + @Deprecated + @DeprecatedApi(since = "2.16.0", forRemoval = "3.0.0") public static FsInfo.Path getFSInfo(NodePath nodePath) throws IOException { FsInfo.Path fsPath = new FsInfo.Path(); fsPath.path = nodePath.path.toString(); @@ -214,11 +223,29 @@ public static FsInfo.Path getFSInfo(NodePath nodePath) throws IOException { fsPath.free = adjustForHugeFilesystems(nodePath.fileStore.getUnallocatedSpace()); fsPath.available = adjustForHugeFilesystems(nodePath.fileStore.getUsableSpace()); fsPath.fileCacheReserved = adjustForHugeFilesystems(nodePath.fileCacheReservedSize.getBytes()); + // The fsPath.fileCacheUtilized cannot be calculated because the information about file cache utilization + // is hold by FileCache which is not available in this context. fsPath.type = nodePath.fileStore.type(); fsPath.mount = nodePath.fileStore.toString(); return fsPath; } + /** + * Retrieves information about the path associated with the given node path. + * + * @param nodePath The node path to retrieve the path information for. + * @param fileCache The FileCache object used for adjusting the file cache utilization. + * @return The path information for the given node path. + * @throws IOException If an I/O error occurs while retrieving the path information. + */ + public static FsInfo.Path getFSInfo(NodePath nodePath, FileCache fileCache) throws IOException { + assert fileCache != null : "FileCache cannot be null"; + FsInfo.Path fsPath = getFSInfo(nodePath); + fsPath.fileCacheUtilized = adjustForHugeFilesystems(fileCache.usage().usage()); + fsPath.available -= (fsPath.fileCacheReserved - fsPath.fileCacheUtilized); + return fsPath; + } + public static long getTotalSize(NodePath nodePath) throws IOException { return adjustForHugeFilesystems(nodePath.fileStore.getTotalSpace()); } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index f7bc96bdfe769..8ff169a6b6244 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -95,6 +95,8 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.TreeMap; import java.util.function.Function; import java.util.stream.Collectors; @@ -291,12 +293,23 @@ public void testSerialization() throws IOException { assertNull(deserializedFs); } else { assertEquals(fs.getTimestamp(), deserializedFs.getTimestamp()); - assertEquals(fs.getTotal().getAvailable(), deserializedFs.getTotal().getAvailable()); - assertEquals(fs.getTotal().getTotal(), deserializedFs.getTotal().getTotal()); - assertEquals(fs.getTotal().getFree(), deserializedFs.getTotal().getFree()); - assertEquals(fs.getTotal().getMount(), deserializedFs.getTotal().getMount()); - assertEquals(fs.getTotal().getPath(), deserializedFs.getTotal().getPath()); - assertEquals(fs.getTotal().getType(), deserializedFs.getTotal().getType()); + compareFsInfo(fs.getTotal(), deserializedFs.getTotal()); + + FsInfo.Path[] fsPaths = StreamSupport.stream( + Spliterators.spliteratorUnknownSize(fs.iterator(), Spliterator.ORDERED), + false + ).toArray(FsInfo.Path[]::new); + + FsInfo.Path[] deserializedFsPaths = StreamSupport.stream( + Spliterators.spliteratorUnknownSize(deserializedFs.iterator(), Spliterator.ORDERED), + false + ).toArray(FsInfo.Path[]::new); + + assertEquals(fsPaths.length, deserializedFsPaths.length); + for (int i = 0; i < fsPaths.length; i++) { + compareFsInfo(fsPaths[i], deserializedFsPaths[i]); + } + FsInfo.IoStats ioStats = fs.getIoStats(); FsInfo.IoStats deserializedIoStats = deserializedFs.getIoStats(); assertEquals(ioStats.getTotalOperations(), deserializedIoStats.getTotalOperations()); @@ -595,11 +608,22 @@ public void testSerialization() throws IOException { } } + private void compareFsInfo(FsInfo.Path path, FsInfo.Path otherPath) { + assertEquals(path.getAvailable(), otherPath.getAvailable()); + assertEquals(path.getTotal(), otherPath.getTotal()); + assertEquals(path.getFree(), otherPath.getFree()); + assertEquals(path.getFileCacheReserved(), otherPath.getFileCacheReserved()); + assertEquals(path.getFileCacheUtilized(), otherPath.getFileCacheUtilized()); + assertEquals(path.getMount(), otherPath.getMount()); + assertEquals(path.getPath(), otherPath.getPath()); + assertEquals(path.getType(), otherPath.getType()); + } + public static NodeStats createNodeStats() throws IOException { return createNodeStats(false); } - public static NodeStats createNodeStats(boolean remoteStoreStats) throws IOException { + private static NodeStats createNodeStats(boolean remoteStoreStats) throws IOException { DiscoveryNode node = new DiscoveryNode( "test_node", buildNewFakeTransportAddress(), @@ -764,6 +788,8 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) throws IOExcep randomBoolean() ? randomAlphaOfLengthBetween(3, 10) : null, randomNonNegativeLong(), randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong(), randomNonNegativeLong() ); } diff --git a/server/src/test/java/org/opensearch/cluster/DiskUsageTests.java b/server/src/test/java/org/opensearch/cluster/DiskUsageTests.java index 5539dd26dd52d..e58574acf3e22 100644 --- a/server/src/test/java/org/opensearch/cluster/DiskUsageTests.java +++ b/server/src/test/java/org/opensearch/cluster/DiskUsageTests.java @@ -157,14 +157,14 @@ public void testFillDiskUsage() { final Map newLeastAvaiableUsages = new HashMap<>(); final Map newMostAvaiableUsages = new HashMap<>(); FsInfo.Path[] node1FSInfo = new FsInfo.Path[] { - new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80), - new FsInfo.Path("/least", "/dev/sdb", 200, 190, 70), - new FsInfo.Path("/most", "/dev/sdc", 300, 290, 280), }; - FsInfo.Path[] node2FSInfo = new FsInfo.Path[] { new FsInfo.Path("/least_most", "/dev/sda", 100, 90, 80), }; + new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80, 0, 0), + new FsInfo.Path("/least", "/dev/sdb", 200, 190, 70, 0, 0), + new FsInfo.Path("/most", "/dev/sdc", 300, 290, 280, 0, 0), }; + FsInfo.Path[] node2FSInfo = new FsInfo.Path[] { new FsInfo.Path("/least_most", "/dev/sda", 100, 90, 80, 0, 0), }; FsInfo.Path[] node3FSInfo = new FsInfo.Path[] { - new FsInfo.Path("/least", "/dev/sda", 100, 90, 70), - new FsInfo.Path("/most", "/dev/sda", 100, 90, 80), }; + new FsInfo.Path("/least", "/dev/sda", 100, 90, 70, 0, 0), + new FsInfo.Path("/most", "/dev/sda", 100, 90, 80, 0, 0), }; List nodeStats = Arrays.asList( new NodeStats( new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), @@ -281,14 +281,14 @@ public void testFillDiskUsageSomeInvalidValues() { final Map newLeastAvailableUsages = new HashMap<>(); final Map newMostAvailableUsages = new HashMap<>(); FsInfo.Path[] node1FSInfo = new FsInfo.Path[] { - new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80), - new FsInfo.Path("/least", "/dev/sdb", -1, -1, -1), - new FsInfo.Path("/most", "/dev/sdc", 300, 290, 280), }; - FsInfo.Path[] node2FSInfo = new FsInfo.Path[] { new FsInfo.Path("/least_most", "/dev/sda", -1, -1, -1), }; + new FsInfo.Path("/middle", "/dev/sda", 100, 90, 80, 0, 0), + new FsInfo.Path("/least", "/dev/sdb", -1, -1, -1, 0, 0), + new FsInfo.Path("/most", "/dev/sdc", 300, 290, 280, 0, 0), }; + FsInfo.Path[] node2FSInfo = new FsInfo.Path[] { new FsInfo.Path("/least_most", "/dev/sda", -1, -1, -1, 0, 0), }; FsInfo.Path[] node3FSInfo = new FsInfo.Path[] { - new FsInfo.Path("/most", "/dev/sda", 100, 90, 70), - new FsInfo.Path("/least", "/dev/sda", 10, -1, 0), }; + new FsInfo.Path("/most", "/dev/sda", 100, 90, 70, 0, 0), + new FsInfo.Path("/least", "/dev/sda", 10, -1, 0, 0, 0), }; List nodeStats = Arrays.asList( new NodeStats( new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 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 e2e09d5ce63fe..5f8691129a235 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -72,8 +72,7 @@ public class FsProbeTests extends OpenSearchTestCase { public void testFsInfo() throws IOException { try (NodeEnvironment env = newNodeEnvironment()) { - FsProbe probe = new FsProbe(env, null); - + FsProbe probe = new FsProbe(env, FileCache.NOOP_FILE_CACHE); FsInfo stats = probe.stats(null); assertNotNull(stats); assertThat(stats.getTimestamp(), greaterThan(0L)); @@ -110,6 +109,15 @@ public void testFsInfo() throws IOException { assertThat(total.total, greaterThan(0L)); assertThat(total.free, greaterThan(0L)); assertThat(total.available, greaterThan(0L)); + // Total file cache (sum over all "paths"): + assertThat(total.fileCacheReserved, equalTo(0L)); + assertThat(total.fileCacheUtilized, equalTo(0L)); + + // The convention for "total" Path object is that some fields are not set + // which means they will not be included in output of toXContent method. + assertNull(total.path); + assertNull(total.mount); + assertNull(total.type); for (FsInfo.Path path : stats) { assertNotNull(path); @@ -204,24 +212,49 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { } public void testFsInfoOverflow() throws Exception { + final String path_r = "/foo/bar"; + final String path_z = "/foo/baz"; final FsInfo.Path pathStats = new FsInfo.Path( - "/foo/bar", + path_r, null, randomNonNegativeLong(), randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong(), randomNonNegativeLong() ); - addUntilOverflow(pathStats, p -> p.total, "total", () -> new FsInfo.Path("/foo/baz", null, randomNonNegativeLong(), 0, 0)); + addUntilOverflow(pathStats, p -> p.total, "total", () -> new FsInfo.Path(path_z, null, randomNonNegativeLong(), 0, 0, 0, 0)); + + addUntilOverflow(pathStats, p -> p.free, "free", () -> new FsInfo.Path(path_z, null, 0, randomNonNegativeLong(), 0, 0, 0)); - addUntilOverflow(pathStats, p -> p.free, "free", () -> new FsInfo.Path("/foo/baz", null, 0, randomNonNegativeLong(), 0)); + addUntilOverflow( + pathStats, + p -> p.available, + "available", + () -> new FsInfo.Path(path_z, null, 0, 0, randomNonNegativeLong(), 0, 0) + ); - addUntilOverflow(pathStats, p -> p.available, "available", () -> new FsInfo.Path("/foo/baz", null, 0, 0, randomNonNegativeLong())); + addUntilOverflow( + pathStats, + p -> p.fileCacheReserved, + "fileCacheReserved", + () -> new FsInfo.Path(path_z, null, 0, 0, 0, randomNonNegativeLong(), 0) + ); + + addUntilOverflow( + pathStats, + p -> p.fileCacheUtilized, + "fileCacheUtilized", + () -> new FsInfo.Path(path_z, null, 0, 0, 0, 0, randomNonNegativeLong()) + ); // even after overflowing these should not be negative assertThat(pathStats.total, greaterThan(0L)); assertThat(pathStats.free, greaterThan(0L)); assertThat(pathStats.available, greaterThan(0L)); + assertThat(pathStats.fileCacheReserved, greaterThan(0L)); + assertThat(pathStats.fileCacheUtilized, greaterThan(0L)); } private void addUntilOverflow( @@ -394,9 +427,25 @@ List readProcDiskStats() throws IOException { public void testAdjustForHugeFilesystems() throws Exception { NodePath np = new FakeNodePath(createTempDir()); - assertThat(FsProbe.getFSInfo(np).total, greaterThanOrEqualTo(0L)); - assertThat(FsProbe.getFSInfo(np).free, greaterThanOrEqualTo(0L)); - assertThat(FsProbe.getFSInfo(np).available, greaterThanOrEqualTo(0L)); + + FsInfo.Path path = FsProbe.getFSInfo(np, FileCache.NOOP_FILE_CACHE); + assertThat(path.total, greaterThanOrEqualTo(0L)); + assertThat(path.free, greaterThanOrEqualTo(0L)); + assertThat(path.available, greaterThanOrEqualTo(0L)); + assertThat(path.fileCacheReserved, greaterThanOrEqualTo(0L)); + assertThat(path.fileCacheUtilized, greaterThanOrEqualTo(0L)); + + /** The following test demonstrates that a call to {@link FsProbe#getFSInfo(NodePath)} + * leaves the file cache utilization value uninitialized which can have unexpected effects. + * Use of that method was deprecated and replaced by {@link FsProbe#getFSInfo(NodePath, FileCache)}. + * {@see https://github.com/opensearch-project/OpenSearch/pull/13232} + */ + path = FsProbe.getFSInfo(np); + assertThat(path.total, greaterThanOrEqualTo(0L)); + assertThat(path.free, greaterThanOrEqualTo(0L)); + assertThat(path.available, greaterThanOrEqualTo(0L)); + assertThat(path.fileCacheReserved, greaterThanOrEqualTo(0L)); + assertThat(path.fileCacheUtilized, greaterThanOrEqualTo(-1L)); // <-- !! } static class FakeNodePath extends NodeEnvironment.NodePath {