Skip to content

Commit f965440

Browse files
danhermannwilliamrandolph
authored andcommitted
[7.7] Do not report negative values for swap sizes (elastic#57324)
1 parent 1fb662d commit f965440

File tree

4 files changed

+46
-10
lines changed

4 files changed

+46
-10
lines changed

server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@
4646
* The {@link OsProbe} class retrieves information about the physical and swap size of the machine
4747
* memory, as well as the system load average and cpu load.
4848
*
49-
* In some exceptional cases, it's possible the underlying native method used by
50-
* {@link #getFreePhysicalMemorySize()} and {@link #getTotalPhysicalMemorySize()} can return a
49+
* In some exceptional cases, it's possible the underlying native methods used by
50+
* {@link #getFreePhysicalMemorySize()}, {@link #getTotalPhysicalMemorySize()},
51+
* {@link #getFreeSwapSpaceSize()}, and {@link #getTotalSwapSpaceSize()} can return a
5152
* negative value. Because of this, we prevent those methods from returning negative values,
5253
* returning 0 instead.
5354
*
@@ -127,12 +128,19 @@ public long getTotalPhysicalMemorySize() {
127128
*/
128129
public long getFreeSwapSpaceSize() {
129130
if (getFreeSwapSpaceSize == null) {
130-
return -1;
131+
logger.warn("getFreeSwapSpaceSize is not available");
132+
return 0;
131133
}
132134
try {
133-
return (long) getFreeSwapSpaceSize.invoke(osMxBean);
135+
final long mem = (long) getFreeSwapSpaceSize.invoke(osMxBean);
136+
if (mem < 0) {
137+
logger.warn("OS reported a negative free swap space size [{}]", mem);
138+
return 0;
139+
}
140+
return mem;
134141
} catch (Exception e) {
135-
return -1;
142+
logger.warn("exception retrieving free swap space size", e);
143+
return 0;
136144
}
137145
}
138146

@@ -141,12 +149,19 @@ public long getFreeSwapSpaceSize() {
141149
*/
142150
public long getTotalSwapSpaceSize() {
143151
if (getTotalSwapSpaceSize == null) {
144-
return -1;
152+
logger.warn("getTotalSwapSpaceSize is not available");
153+
return 0;
145154
}
146155
try {
147-
return (long) getTotalSwapSpaceSize.invoke(osMxBean);
156+
final long mem = (long) getTotalSwapSpaceSize.invoke(osMxBean);
157+
if (mem < 0) {
158+
logger.warn("OS reported a negative total swap space size [{}]", mem);
159+
return 0;
160+
}
161+
return mem;
148162
} catch (Exception e) {
149-
return -1;
163+
logger.warn("exception retrieving total swap space size", e);
164+
return 0;
150165
}
151166
}
152167

server/src/main/java/org/elasticsearch/monitor/os/OsStats.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,17 +189,23 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
189189

190190
public static class Swap implements Writeable, ToXContentFragment {
191191

192+
private static final Logger logger = LogManager.getLogger(Swap.class);
193+
192194
private final long total;
193195
private final long free;
194196

195197
public Swap(long total, long free) {
198+
assert total >= 0 : "expected total swap to be positive, got: " + total;
199+
assert free >= 0 : "expected free swap to be positive, got: " + total;
196200
this.total = total;
197201
this.free = free;
198202
}
199203

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

205211
@Override
@@ -213,6 +219,17 @@ public ByteSizeValue getFree() {
213219
}
214220

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

server/src/test/java/org/elasticsearch/monitor/os/OsStatsTests.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,12 @@ public void testSerialization() throws IOException {
8484
}
8585

8686
public void testGetUsedMemoryWithZeroTotal() {
87-
OsStats.Mem mem = new OsStats.Mem(0, 1);
87+
OsStats.Mem mem = new OsStats.Mem(0, randomNonNegativeLong());
8888
assertThat(mem.getUsed().getBytes(), equalTo(0L));
8989
}
9090

91+
public void testGetUsedSwapWithZeroTotal() {
92+
OsStats.Swap swap = new OsStats.Swap(0, randomNonNegativeLong());
93+
assertThat(swap.getUsed().getBytes(), equalTo(0L));
94+
}
9195
}

x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ private static NodeStats mockNodeStats() {
335335
"_memory_ctrl_group", "2000000000", "1000000000");
336336

337337
final OsStats.Mem osMem = new OsStats.Mem(0, 0);
338-
final OsStats.Swap osSwap = new OsStats.Swap(no, no);
338+
final OsStats.Swap osSwap = new OsStats.Swap(0, 0);
339339
final OsStats os = new OsStats(no, osCpu, osMem, osSwap, osCgroup);
340340

341341
// Process

0 commit comments

Comments
 (0)