Skip to content

Commit

Permalink
[8.x] OsStats must be lenient with bad data from older nodes (#73610)
Browse files Browse the repository at this point in the history
We've had a series of bug fixes for cases where an OsProbe gives negative
values, most often just -1, to the OsStats class. We added assertions to catch
cases where we were initializing OsStats with bad values. Unfortunately, these
fixes turned to not be backwards-compatible. In this commit, we simply coerce
bad values to 0 when data is coming from nodes that don't have the relevant bug
fixes.

Relevant PRs:
* #42725
* #56435
* #57317

Fixes #73459
  • Loading branch information
williamrandolph authored Jun 1, 2021
1 parent 3d80e77 commit 80ea64c
Showing 1 changed file with 25 additions and 8 deletions.
33 changes: 25 additions & 8 deletions server/src/main/java/org/elasticsearch/monitor/os/OsStats.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
Expand Down Expand Up @@ -184,10 +185,18 @@ public Swap(long total, long 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;
if (in.getVersion().onOrAfter(Version.V_7_8_0)) {
this.total = in.readLong();
assert this.total >= 0 : "expected total swap to be positive, got: " + total;
this.free = in.readLong();
assert this.free >= 0 : "expected free swap to be positive, got: " + total;
} else {
// If we have a node in the cluster without the bug fix for
// negative memory values, we need to coerce negative values to 0 here.
// The relevant bug fix was added for 7.8.0 in https://github.com/elastic/elasticsearch/pull/57317
this.total = Math.max(0, in.readLong());
this.free = Math.max(0, in.readLong());
}
}

@Override
Expand Down Expand Up @@ -247,10 +256,18 @@ public Mem(long total, long free) {
}

public Mem(StreamInput in) throws IOException {
this.total = in.readLong();
assert total >= 0 : "expected total memory to be positive, got: " + total;
this.free = in.readLong();
assert free >= 0 : "expected free memory to be positive, got: " + total;
if (in.getVersion().onOrAfter(Version.V_7_2_0)) {
this.total = in.readLong();
assert total >= 0 : "expected total memory to be positive, got: " + total;
this.free = in.readLong();
assert free >= 0 : "expected free memory to be positive, got: " + total;
} else {
// If we have a node in the cluster without the bug fix for
// negative memory values, we need to coerce negative values to 0 here.
// The relevant bug fix was added for 7.2.0 in https://github.com/elastic/elasticsearch/pull/42725
this.total = Math.max(0, in.readLong());
this.free = Math.max(0, in.readLong());
}
}

@Override
Expand Down

0 comments on commit 80ea64c

Please sign in to comment.