Skip to content

Commit

Permalink
Adding tests for file cache statistics response
Browse files Browse the repository at this point in the history
Addming missing tests for file caches in NodeStats FsInfo.
This commit changes the test value for `cache_utilized` field.
Going forward both file cache fields are present in the node stats REST response.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
  • Loading branch information
lukas-vlcek committed Jun 11, 2024
1 parent 4349023 commit 29a9ba3
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# 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 fields were added in 2.7 but #13232 was not back-ported yet"
features: [arbitrary_key]

- do:
nodes.info: {}
- set:
nodes._arbitrary_key_: node_id

- do:
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 }
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 16 additions & 12 deletions server/src/main/java/org/opensearch/env/NodeEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,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
Expand Down Expand Up @@ -171,18 +172,21 @@ Path resolve(String uuid) {

@Override
public String toString() {
return "NodePath{"
+ "path="
+ path
+ ", indicesPath="
+ indicesPath
+ ", fileStore="
+ fileStore
+ ", majorDeviceNumber="
+ majorDeviceNumber
+ ", minorDeviceNumber="
+ minorDeviceNumber
+ '}';
StringBuilder sb = new StringBuilder().append("NodePath{")
.append("path=")
.append(path)
.append(", indicesPath=")
.append(indicesPath)
.append(", fileStore=")
.append(fileStore)
.append(", fileCachePath=")
.append(fileCachePath)
.append(", majorDeviceNumber=")
.append(majorDeviceNumber)
.append(", minorDeviceNumber=")
.append(minorDeviceNumber)
.append("}");
return sb.toString();
}

}
Expand Down
15 changes: 13 additions & 2 deletions server/src/main/java/org/opensearch/monitor/fs/FsInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,23 @@ public static class Path implements Writeable, ToXContentObject {

public Path() {}

public Path(String path, @Nullable String mount, long total, long free, long available) {
// Used only in tests
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;
}

/**
Expand Down Expand Up @@ -211,7 +222,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (fileCacheReserved != -1) {
builder.humanReadableField(Fields.CACHE_RESERVED_IN_BYTES, Fields.CACHE_RESERVED, getFileCacheReserved());
}
if (fileCacheUtilized != 0) {
if (fileCacheUtilized != -1) {
builder.humanReadableField(Fields.CACHE_UTILIZED_IN_BYTES, Fields.CACHE_UTILIZED, getFileCacheUtilized());

Check warning on line 226 in server/src/main/java/org/opensearch/monitor/fs/FsInfo.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/monitor/fs/FsInfo.java#L226

Added line #L226 was not covered by tests
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -766,6 +790,8 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) throws IOExcep
randomBoolean() ? randomAlphaOfLengthBetween(3, 10) : null,
randomNonNegativeLong(),
randomNonNegativeLong(),
randomNonNegativeLong(),
randomNonNegativeLong(),
randomNonNegativeLong()
);
}
Expand Down
24 changes: 12 additions & 12 deletions server/src/test/java/org/opensearch/cluster/DiskUsageTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,14 @@ public void testFillDiskUsage() {
final Map<String, DiskUsage> newLeastAvaiableUsages = new HashMap<>();
final Map<String, DiskUsage> 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> nodeStats = Arrays.asList(
new NodeStats(
new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT),
Expand Down Expand Up @@ -281,14 +281,14 @@ public void testFillDiskUsageSomeInvalidValues() {
final Map<String, DiskUsage> newLeastAvailableUsages = new HashMap<>();
final Map<String, DiskUsage> 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> nodeStats = Arrays.asList(
new NodeStats(
new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT),
Expand Down
33 changes: 29 additions & 4 deletions server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,24 +163,49 @@ public void testFsCacheInfo() 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("/foo/baz", null, 0, randomNonNegativeLong(), 0));
addUntilOverflow(pathStats, p -> p.free, "free", () -> new FsInfo.Path(path_z, null, 0, randomNonNegativeLong(), 0, 0, 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(
Expand Down

0 comments on commit 29a9ba3

Please sign in to comment.