Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] Do not report negative values for swap sizes #57353

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@
* The {@link OsProbe} class retrieves information about the physical and swap size of the machine
* memory, as well as the system load average and cpu load.
*
* In some exceptional cases, it's possible the underlying native method used by
* {@link #getFreePhysicalMemorySize()} and {@link #getTotalPhysicalMemorySize()} can return a
* In some exceptional cases, it's possible the underlying native methods used by
* {@link #getFreePhysicalMemorySize()}, {@link #getTotalPhysicalMemorySize()},
* {@link #getFreeSwapSpaceSize()}, and {@link #getTotalSwapSpaceSize()} can return a
* negative value. Because of this, we prevent those methods from returning negative values,
* returning 0 instead.
*
Expand Down Expand Up @@ -127,12 +128,19 @@ public long getTotalPhysicalMemorySize() {
*/
public long getFreeSwapSpaceSize() {
if (getFreeSwapSpaceSize == null) {
return -1;
logger.warn("getFreeSwapSpaceSize is not available");
return 0;
}
try {
return (long) getFreeSwapSpaceSize.invoke(osMxBean);
final long mem = (long) getFreeSwapSpaceSize.invoke(osMxBean);
if (mem < 0) {
logger.warn("OS reported a negative free swap space size [{}]", mem);
return 0;
}
return mem;
} catch (Exception e) {
return -1;
logger.warn("exception retrieving free swap space size", e);
return 0;
}
}

Expand All @@ -141,12 +149,19 @@ public long getFreeSwapSpaceSize() {
*/
public long getTotalSwapSpaceSize() {
if (getTotalSwapSpaceSize == null) {
return -1;
logger.warn("getTotalSwapSpaceSize is not available");
return 0;
}
try {
return (long) getTotalSwapSpaceSize.invoke(osMxBean);
final long mem = (long) getTotalSwapSpaceSize.invoke(osMxBean);
if (mem < 0) {
logger.warn("OS reported a negative total swap space size [{}]", mem);
return 0;
}
return mem;
} catch (Exception e) {
return -1;
logger.warn("exception retrieving total swap space size", e);
return 0;
}
}

Expand Down
17 changes: 17 additions & 0 deletions server/src/main/java/org/elasticsearch/monitor/os/OsStats.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,23 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws

public static class Swap implements Writeable, ToXContentFragment {

private static final Logger logger = LogManager.getLogger(Swap.class);

private final long total;
private final long free;

public Swap(long total, long free) {
assert total >= 0 : "expected total swap to be positive, got: " + total;
assert free >= 0 : "expected free swap to be positive, got: " + total;
this.total = total;
this.free = free;
}

public Swap(StreamInput in) throws IOException {
this.total = in.readLong();
assert total >= 0 : "expected total swap to be positive, got: " + total;
this.free = in.readLong();
assert free >= 0 : "expected free swap to be positive, got: " + total;
}

@Override
Expand All @@ -207,6 +213,17 @@ public ByteSizeValue getFree() {
}

public ByteSizeValue getUsed() {
if (total == 0) {
// The work in https://github.com/elastic/elasticsearch/pull/42725 established that total memory
// can be reported as negative in some cases. Swap can similarly be reported as negative and in
// those cases, we force it to zero in which case we can no longer correctly report the used swap
// as (total-free) and should report it as zero.
//
// We intentionally check for (total == 0) rather than (total - free < 0) so as not to hide
// cases where (free > total) which would be a different bug.
logger.warn("cannot compute used swap when total swap is 0 and free swap is " + free);
return new ByteSizeValue(0);
}
return new ByteSizeValue(total - free);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,12 @@ public void testSerialization() throws IOException {
}

public void testGetUsedMemoryWithZeroTotal() {
OsStats.Mem mem = new OsStats.Mem(0, 1);
OsStats.Mem mem = new OsStats.Mem(0, randomNonNegativeLong());
assertThat(mem.getUsed().getBytes(), equalTo(0L));
}

public void testGetUsedSwapWithZeroTotal() {
OsStats.Swap swap = new OsStats.Swap(0, randomNonNegativeLong());
assertThat(swap.getUsed().getBytes(), equalTo(0L));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ private static NodeStats mockNodeStats() {
"_memory_ctrl_group", "2000000000", "1000000000");

final OsStats.Mem osMem = new OsStats.Mem(0, 0);
final OsStats.Swap osSwap = new OsStats.Swap(no, no);
final OsStats.Swap osSwap = new OsStats.Swap(0, 0);
final OsStats os = new OsStats(no, osCpu, osMem, osSwap, osCgroup);

// Process
Expand Down