Skip to content

Commit

Permalink
The value of fileCacheUtilized should use standard init value
Browse files Browse the repository at this point in the history
In FsInfo.Path object the value of fileCacheUtilized is now initialized
like any other fields, that means it is -1, which means the value hasn't been
populated yet.

To make this possible it was necessary to fix FsProbeTest and use
a real FileCache object to enable internal logic to handle initialization
and safeguarding of the value properly.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
  • Loading branch information
lukas-vlcek committed Jun 19, 2024
1 parent 29a9ba3 commit f375977
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/monitor/fs/FsInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static class Path implements Writeable, ToXContentObject {
long free = -1;
long available = -1;
long fileCacheReserved = -1;
long fileCacheUtilized = 0;
long fileCacheUtilized = -1;

public Path() {}

Expand Down
5 changes: 4 additions & 1 deletion server/src/main/java/org/opensearch/monitor/fs/FsProbe.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public FsInfo stats(FsInfo previous) throws IOException {
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) {
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());
paths[i].available -= (paths[i].fileCacheReserved - paths[i].fileCacheUtilized);
Expand Down Expand Up @@ -206,6 +206,9 @@ 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());
// fsPath.fileCacheUtilized = adjustForHugeFilesystems(...);
// We can not do this ^^ here because information about utilization of file cache is hold by FileCache
// which is not accessible here and since this method is static we can not assume relevant context.
fsPath.type = nodePath.fileStore.type();
fsPath.mount = nodePath.fileStore.toString();
return fsPath;
Expand Down
22 changes: 21 additions & 1 deletion server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,17 @@ public class FsProbeTests extends OpenSearchTestCase {
public void testFsInfo() throws IOException {

try (NodeEnvironment env = newNodeEnvironment()) {
FsProbe probe = new FsProbe(env, null);
// Question: Shall we expose a public method in FileCacheTests to enable creation of FileCache
// so that it can be used by other testing classes?
int CONCURRENCY_LEVEL = 16; // not important
int CAPACITY = 1 * 1024; // not important
FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(
CAPACITY,
CONCURRENCY_LEVEL,
new NoopCircuitBreaker(CircuitBreaker.REQUEST)
);
// We need to pass a real FileCache object to FsProbe ctor to have it safeguard "path.fileCacheUtilized" values properly!
FsProbe probe = new FsProbe(env, fileCache);

FsInfo stats = probe.stats(null);
assertNotNull(stats);
Expand Down Expand Up @@ -110,6 +120,16 @@ public void testFsInfo() throws IOException {
assertThat(total.free, greaterThan(0L));
assertThat(total.available, greaterThan(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);

// Total file cache (sum over all "paths"):
assertEquals(total.getFileCacheReserved().getBytes(), 0);
assertEquals(total.getFileCacheUtilized().getBytes(), 0);

for (FsInfo.Path path : stats) {
assertNotNull(path);
assertThat(path.getPath(), is(not(emptyOrNullString())));
Expand Down

0 comments on commit f375977

Please sign in to comment.