From 3c7c8723ff72379d440a4c214c8ff029b5b27942 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 20 Oct 2016 15:39:46 -0400 Subject: [PATCH] Cleanup load average handling This commit cleans up the code handling load averages in OsProbe: - remove support for BSD; we do not support this OS - add Javadocs - strengthen assertions and testing - add debug logging for exceptional situation Relates #21037 --- .../org/elasticsearch/monitor/os/OsProbe.java | 78 +++++++++++++------ .../elasticsearch/bootstrap/security.policy | 3 - .../monitor/os/OsProbeTests.java | 28 +++++-- 3 files changed, 76 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/monitor/os/OsProbe.java b/core/src/main/java/org/elasticsearch/monitor/os/OsProbe.java index 08abfc05f1d..847e38bbf8c 100644 --- a/core/src/main/java/org/elasticsearch/monitor/os/OsProbe.java +++ b/core/src/main/java/org/elasticsearch/monitor/os/OsProbe.java @@ -19,9 +19,13 @@ package org.elasticsearch.monitor.os; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.logging.log4j.util.Supplier; import org.apache.lucene.util.Constants; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.monitor.Probes; import java.io.IOException; @@ -108,44 +112,68 @@ public class OsProbe { } /** - * Returns the system load averages + * The system load averages as an array. + * + * On Windows, this method returns {@code null}. + * + * On Linux, this method should return the 1, 5, and 15-minute load + * averages. If obtaining these values from {@code /proc/loadavg} + * fails, the method will fallback to obtaining the 1-minute load + * average. + * + * On macOS, this method should return the 1-minute load average. + * + * @return the available system load averages or {@code null} */ - public double[] getSystemLoadAverage() { - if (Constants.LINUX || Constants.FREE_BSD) { - final String procLoadAvg = Constants.LINUX ? "/proc/loadavg" : "/compat/linux/proc/loadavg"; - double[] loadAverage = readProcLoadavg(procLoadAvg); - if (loadAverage != null) { - return loadAverage; + final double[] getSystemLoadAverage() { + if (Constants.WINDOWS) { + return null; + } else if (Constants.LINUX) { + final String procLoadAvg = readProcLoadavg(); + if (procLoadAvg != null) { + assert procLoadAvg.matches("(\\d+\\.\\d+\\s+){3}\\d+/\\d+\\s+\\d+"); + final String[] fields = procLoadAvg.split("\\s+"); + try { + return new double[]{Double.parseDouble(fields[0]), Double.parseDouble(fields[1]), Double.parseDouble(fields[2])}; + } catch (final NumberFormatException e) { + logger.debug((Supplier) () -> new ParameterizedMessage("error parsing /proc/loadavg [{}]", procLoadAvg), e); + } } // fallback } - if (Constants.WINDOWS) { - return null; - } + if (getSystemLoadAverage == null) { return null; } try { - double oneMinuteLoadAverage = (double) getSystemLoadAverage.invoke(osMxBean); + final double oneMinuteLoadAverage = (double) getSystemLoadAverage.invoke(osMxBean); return new double[] { oneMinuteLoadAverage >= 0 ? oneMinuteLoadAverage : -1, -1, -1 }; - } catch (Exception e) { + } catch (final Exception e) { + logger.debug("error obtaining system load average", e); return null; } } - @SuppressForbidden(reason = "access /proc") - private static double[] readProcLoadavg(String procLoadavg) { + /** + * The line from {@code /proc/loadavg}. The first three fields are + * the load averages averaged over 1, 5, and 15 minutes. The fourth + * field is two numbers separated by a slash, the first is the + * number of currently runnable scheduling entities, the second is + * the number of scheduling entities on the system. The fifth field + * is the PID of the most recently created process. + * + * @return the line from {@code /proc/loadavg} or {@code null} + */ + @SuppressForbidden(reason = "access /proc/loadavg") + String readProcLoadavg() { try { - List lines = Files.readAllLines(PathUtils.get(procLoadavg)); - if (!lines.isEmpty()) { - String[] fields = lines.get(0).split("\\s+"); - return new double[] { Double.parseDouble(fields[0]), Double.parseDouble(fields[1]), Double.parseDouble(fields[2]) }; - } - } catch (IOException e) { - // do not fail Elasticsearch if something unexpected - // happens here + final List lines = Files.readAllLines(PathUtils.get("/proc/loadavg")); + assert lines != null && lines.size() == 1; + return lines.get(0); + } catch (final IOException e) { + logger.debug("error reading /proc/loadavg", e); + return null; } - return null; } public short getSystemCpuPercent() { @@ -160,9 +188,11 @@ public class OsProbe { return OsProbeHolder.INSTANCE; } - private OsProbe() { + OsProbe() { } + private final Logger logger = ESLoggerFactory.getLogger(getClass()); + public OsInfo osInfo(long refreshInterval, int allocatedProcessors) { return new OsInfo(refreshInterval, Runtime.getRuntime().availableProcessors(), allocatedProcessors, Constants.OS_NAME, Constants.OS_ARCH, Constants.OS_VERSION); diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index 97ccfb31bf2..4c5c2768987 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -116,9 +116,6 @@ grant { // load averages on Linux permission java.io.FilePermission "/proc/loadavg", "read"; - // load averages on FreeBSD - permission java.io.FilePermission "/compat/linux/proc/loadavg", "read"; - // read max virtual memory areas permission java.io.FilePermission "/proc/sys/vm/max_map_count", "read"; diff --git a/core/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java b/core/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java index da9169dcdbb..9cef6671c3e 100644 --- a/core/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java +++ b/core/src/test/java/org/elasticsearch/monitor/os/OsProbeTests.java @@ -66,12 +66,6 @@ public class OsProbeTests extends ESTestCase { assertThat(loadAverage[0], greaterThanOrEqualTo((double) 0)); assertThat(loadAverage[1], greaterThanOrEqualTo((double) 0)); assertThat(loadAverage[2], greaterThanOrEqualTo((double) 0)); - } else if (Constants.FREE_BSD) { - // five- and fifteen-minute load averages not available if linprocfs is not mounted at /compat/linux/proc - assertNotNull(loadAverage); - assertThat(loadAverage[0], greaterThanOrEqualTo((double) 0)); - assertThat(loadAverage[1], anyOf(equalTo((double) -1), greaterThanOrEqualTo((double) 0))); - assertThat(loadAverage[2], anyOf(equalTo((double) -1), greaterThanOrEqualTo((double) 0))); } else if (Constants.MAC_OS_X) { // one minute load average is available, but 10-minute and 15-minute load averages are not assertNotNull(loadAverage); @@ -109,4 +103,26 @@ public class OsProbeTests extends ESTestCase { assertThat(stats.getSwap().getUsed().getBytes(), equalTo(0L)); } } + + public void testGetSystemLoadAverage() { + assumeTrue("test runs on Linux only", Constants.LINUX); + + final OsProbe probe = new OsProbe() { + @Override + String readProcLoadavg() { + return "1.51 1.69 1.99 3/417 23251"; + } + }; + + final double[] systemLoadAverage = probe.getSystemLoadAverage(); + + assertNotNull(systemLoadAverage); + assertThat(systemLoadAverage.length, equalTo(3)); + + // avoid silliness with representing doubles + assertThat(systemLoadAverage[0], equalTo(Double.parseDouble("1.51"))); + assertThat(systemLoadAverage[1], equalTo(Double.parseDouble("1.69"))); + assertThat(systemLoadAverage[2], equalTo(Double.parseDouble("1.99"))); + } + }