Skip to content

Commit

Permalink
Fix fs info reporting negative available size (#11573)
Browse files Browse the repository at this point in the history
* fix fs info reporting negative available size

Signed-off-by: panguixin <panguixin@bytedance.com>

* change log

Signed-off-by: panguixin <panguixin@bytedance.com>

* fix test

Signed-off-by: panguixin <panguixin@bytedance.com>

* fix test

Signed-off-by: panguixin <panguixin@bytedance.com>

* spotless

Signed-off-by: panguixin <panguixin@bytedance.com>

---------

Signed-off-by: panguixin <panguixin@bytedance.com>
Signed-off-by: Andrew Ross <andrross@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 1da19d3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] and andrross committed Jun 24, 2024
1 parent 1bb42ec commit 4ade454
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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))
- Add ListPitInfo::getKeepAlive() getter ([#14495](https://github.com/opensearch-project/OpenSearch/pull/14495))

### Security
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/opensearch/monitor/fs/FsProbe.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 41 additions & 0 deletions server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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;
Expand Down Expand Up @@ -162,6 +163,46 @@ 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)) {
// 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(),
16,
new NoopCircuitBreaker(CircuitBreaker.REQUEST)
);

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);

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);
} else {
assertTrue(path.available > 0L);
}
}
}
}

public void testFsInfoOverflow() throws Exception {
final FsInfo.Path pathStats = new FsInfo.Path(
"/foo/bar",
Expand Down

0 comments on commit 4ade454

Please sign in to comment.