Avoid overflow when computing total FS stats

When adding filesystem stats from individual filesystems, free and
available can overflow. This commit guards against this by adjusting
these situations to Long.MAX_VALUE.

Relates #23641
This commit is contained in:
Jason Tedor 2017-03-18 20:02:18 -04:00 committed by GitHub
parent 23f0bf6b68
commit b23adb6d15
2 changed files with 58 additions and 18 deletions

View File

@ -138,8 +138,8 @@ public class FsInfo implements Iterable<FsInfo.Path>, Writeable, ToXContent {
public void add(Path path) { public void add(Path path) {
total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total)); total = FsProbe.adjustForHugeFilesystems(addLong(total, path.total));
free = addLong(free, path.free); free = FsProbe.adjustForHugeFilesystems(addLong(free, path.free));
available = addLong(available, path.available); available = FsProbe.adjustForHugeFilesystems(addLong(available, path.available));
if (path.spins != null && path.spins.booleanValue()) { if (path.spins != null && path.spins.booleanValue()) {
// Spinning is contagious! // Spinning is contagious!
spins = Boolean.TRUE; spins = Boolean.TRUE;

View File

@ -36,6 +36,8 @@ import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.Supplier;
import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThan;
@ -93,26 +95,64 @@ public class FsProbeTests extends ESTestCase {
} }
public void testFsInfoOverflow() throws Exception { public void testFsInfoOverflow() throws Exception {
FsInfo.Path pathStats = new FsInfo.Path("/foo/bar", null, final FsInfo.Path pathStats =
randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); new FsInfo.Path(
"/foo/bar",
null,
randomNonNegativeLong(),
randomNonNegativeLong(),
randomNonNegativeLong());
// While not overflowing, keep adding addUntilOverflow(
FsInfo.Path pathToAdd = new FsInfo.Path("/foo/baz", null, pathStats,
randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); p -> p.total,
while ((pathStats.total + pathToAdd.total) > 0) { "total",
// Add itself as a path, to increase the total bytes until it overflows () -> new FsInfo.Path("/foo/baz", null, randomNonNegativeLong(), 0, 0));
logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total);
pathStats.add(pathToAdd);
pathToAdd = new FsInfo.Path("/foo/baz", null,
randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
}
logger.info("--> adding {} bytes to {}, will be: {}", pathToAdd.total, pathStats.total, pathToAdd.total + pathStats.total); addUntilOverflow(
assertThat(pathStats.total + pathToAdd.total, lessThan(0L)); pathStats,
pathStats.add(pathToAdd); p -> p.free,
"free",
() -> new FsInfo.Path("/foo/baz", null, 0, randomNonNegativeLong(), 0));
// Even after overflowing, it should not be negative addUntilOverflow(
pathStats,
p -> p.available,
"available",
() -> new FsInfo.Path("/foo/baz", null, 0, 0, randomNonNegativeLong()));
// even after overflowing these should not be negative
assertThat(pathStats.total, greaterThan(0L)); assertThat(pathStats.total, greaterThan(0L));
assertThat(pathStats.free, greaterThan(0L));
assertThat(pathStats.available, greaterThan(0L));
}
private void addUntilOverflow(
final FsInfo.Path pathStats,
final Function<FsInfo.Path, Long> getter,
final String field,
final Supplier<FsInfo.Path> supplier) {
FsInfo.Path pathToAdd = supplier.get();
while ((getter.apply(pathStats) + getter.apply(pathToAdd)) > 0) {
// add a path to increase the total bytes until it overflows
logger.info(
"--> adding {} bytes to {}, {} will be: {}",
getter.apply(pathToAdd),
getter.apply(pathStats),
field,
getter.apply(pathStats) + getter.apply(pathToAdd));
pathStats.add(pathToAdd);
pathToAdd = supplier.get();
}
// this overflows
logger.info(
"--> adding {} bytes to {}, {} will be: {}",
getter.apply(pathToAdd),
getter.apply(pathStats),
field,
getter.apply(pathStats) + getter.apply(pathToAdd));
assertThat(getter.apply(pathStats) + getter.apply(pathToAdd), lessThan(0L));
pathStats.add(pathToAdd);
} }
public void testIoStats() { public void testIoStats() {