honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-25 01:19:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor bootstrap check results and error messages

Browse Source 
This commit refactors the bootstrap checks into a single result object
that encapsulates whether or not the check passed, and a failure message
if the check failed. This simpifies the checks, and enables the messages
to more easily be based on the state used to discern whether or not the
check passed.

Relates #26637
This commit is contained in:
Jason Tedor 2017-09-13 21:30:27 -04:00 committed by GitHub
parent b4de2a6f28
commit ca6bce75da
5 changed files with 186 additions and 196 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java
View File

@ -19,27 +19,61 @@
package org.elasticsearch.bootstrap;
import java.util.Objects;
/**
* Encapsulates a bootstrap check.
*/
public interface BootstrapCheck {
/**
* Test if the node fails the check.
*
* @param context the bootstrap context for more sophisticated checks
* @return {@code true} if the node failed the check
* Encapsulate the result of a bootstrap check.
*/
boolean check(BootstrapContext context);
final class BootstrapCheckResult {
private final String message;
private static final BootstrapCheckResult SUCCESS = new BootstrapCheckResult(null);
public static BootstrapCheckResult success() {
return SUCCESS;
}
public static BootstrapCheckResult failure(final String message) {
Objects.requireNonNull(message);
return new BootstrapCheckResult(message);
}
private BootstrapCheckResult(final String message) {
this.message = message;
}
public boolean isSuccess() {
return this == SUCCESS;
}
public boolean isFailure() {
return !isSuccess();
}
public String getMessage() {
assert isFailure();
assert message != null;
return message;
}
}
/**
* The error message for a failed check.
* Test if the node fails the check.
*
* @return the error message on check failure
* @param context the bootstrap context
* @return the result of the bootstrap check
*/
String errorMessage();
BootstrapCheckResult check(BootstrapContext context);
default boolean alwaysEnforce() {
return false;
}
}

271
core/src/main/java/org/elasticsearch/bootstrap/BootstrapChecks.java
View File

@ -137,11 +137,12 @@ final class BootstrapChecks {
}
for (final BootstrapCheck check : checks) {
if (check.check(context)) {
final BootstrapCheck.BootstrapCheckResult result = check.check(context);
if (result.isFailure()) {
if (!(enforceLimits || enforceBootstrapChecks) && !check.alwaysEnforce()) {
ignoredErrors.add(check.errorMessage());
ignoredErrors.add(result.getMessage());
} else {
errors.add(check.errorMessage());
errors.add(result.getMessage());
}
}
}
@ -215,21 +216,20 @@ final class BootstrapChecks {
static class HeapSizeCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
public BootstrapCheckResult check(BootstrapContext context) {
final long initialHeapSize = getInitialHeapSize();
final long maxHeapSize = getMaxHeapSize();
return initialHeapSize != 0 && maxHeapSize != 0 && initialHeapSize != maxHeapSize;
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"initial heap size [%d] not equal to maximum heap size [%d]; " +
"this can cause resize pauses and prevents mlockall from locking the entire heap",
getInitialHeapSize(),
getMaxHeapSize()
);
if (initialHeapSize != 0 && maxHeapSize != 0 && initialHeapSize != maxHeapSize) {
final String message = String.format(
Locale.ROOT,
"initial heap size [%d] not equal to maximum heap size [%d]; " +
"this can cause resize pauses and prevents mlockall from locking the entire heap",
getInitialHeapSize(),
getMaxHeapSize());
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -271,19 +271,18 @@ final class BootstrapChecks {
this.limit = limit;
}
public final boolean check(BootstrapContext context) {
public final BootstrapCheckResult check(BootstrapContext context) {
final long maxFileDescriptorCount = getMaxFileDescriptorCount();
return maxFileDescriptorCount != -1 && maxFileDescriptorCount < limit;
}
@Override
public final String errorMessage() {
return String.format(
Locale.ROOT,
"max file descriptors [%d] for elasticsearch process is too low, increase to at least [%d]",
getMaxFileDescriptorCount(),
limit
);
if (maxFileDescriptorCount != -1 && maxFileDescriptorCount < limit) {
final String message = String.format(
Locale.ROOT,
"max file descriptors [%d] for elasticsearch process is too low, increase to at least [%d]",
getMaxFileDescriptorCount(),
limit);
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -296,13 +295,12 @@ final class BootstrapChecks {
static class MlockallCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return BootstrapSettings.MEMORY_LOCK_SETTING.get(context.settings) && !isMemoryLocked();
}
@Override
public String errorMessage() {
return "memory locking requested for elasticsearch process but memory is not locked";
public BootstrapCheckResult check(BootstrapContext context) {
if (BootstrapSettings.MEMORY_LOCK_SETTING.get(context.settings) && !isMemoryLocked()) {
return BootstrapCheckResult.failure("memory locking requested for elasticsearch process but memory is not locked");
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -318,18 +316,18 @@ final class BootstrapChecks {
private static final long MAX_NUMBER_OF_THREADS_THRESHOLD = 1 << 12;
@Override
public boolean check(BootstrapContext context) {
return getMaxNumberOfThreads() != -1 && getMaxNumberOfThreads() < MAX_NUMBER_OF_THREADS_THRESHOLD;
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"max number of threads [%d] for user [%s] is too low, increase to at least [%d]",
getMaxNumberOfThreads(),
BootstrapInfo.getSystemProperties().get("user.name"),
MAX_NUMBER_OF_THREADS_THRESHOLD);
public BootstrapCheckResult check(BootstrapContext context) {
if (getMaxNumberOfThreads() != -1 && getMaxNumberOfThreads() < MAX_NUMBER_OF_THREADS_THRESHOLD) {
final String message = String.format(
Locale.ROOT,
"max number of threads [%d] for user [%s] is too low, increase to at least [%d]",
getMaxNumberOfThreads(),
BootstrapInfo.getSystemProperties().get("user.name"),
MAX_NUMBER_OF_THREADS_THRESHOLD);
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -342,17 +340,17 @@ final class BootstrapChecks {
static class MaxSizeVirtualMemoryCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return getMaxSizeVirtualMemory() != Long.MIN_VALUE && getMaxSizeVirtualMemory() != getRlimInfinity();
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"max size virtual memory [%d] for user [%s] is too low, increase to [unlimited]",
getMaxSizeVirtualMemory(),
BootstrapInfo.getSystemProperties().get("user.name"));
public BootstrapCheckResult check(BootstrapContext context) {
if (getMaxSizeVirtualMemory() != Long.MIN_VALUE && getMaxSizeVirtualMemory() != getRlimInfinity()) {
final String message = String.format(
Locale.ROOT,
"max size virtual memory [%d] for user [%s] is too low, increase to [unlimited]",
getMaxSizeVirtualMemory(),
BootstrapInfo.getSystemProperties().get("user.name"));
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -373,18 +371,18 @@ final class BootstrapChecks {
static class MaxFileSizeCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
public BootstrapCheckResult check(BootstrapContext context) {
final long maxFileSize = getMaxFileSize();
return maxFileSize != Long.MIN_VALUE && maxFileSize != getRlimInfinity();
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"max file size [%d] for user [%s] is too low, increase to [unlimited]",
getMaxFileSize(),
BootstrapInfo.getSystemProperties().get("user.name"));
if (maxFileSize != Long.MIN_VALUE && maxFileSize != getRlimInfinity()) {
final String message = String.format(
Locale.ROOT,
"max file size [%d] for user [%s] is too low, increase to [unlimited]",
getMaxFileSize(),
BootstrapInfo.getSystemProperties().get("user.name"));
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
long getRlimInfinity() {
@ -402,17 +400,17 @@ final class BootstrapChecks {
private static final long LIMIT = 1 << 18;
@Override
public boolean check(BootstrapContext context) {
return getMaxMapCount() != -1 && getMaxMapCount() < LIMIT;
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"max virtual memory areas vm.max_map_count [%d] is too low, increase to at least [%d]",
getMaxMapCount(),
LIMIT);
public BootstrapCheckResult check(BootstrapContext context) {
if (getMaxMapCount() != -1 && getMaxMapCount() < LIMIT) {
final String message = String.format(
Locale.ROOT,
"max virtual memory areas vm.max_map_count [%d] is too low, increase to at least [%d]",
getMaxMapCount(),
LIMIT);
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -467,8 +465,16 @@ final class BootstrapChecks {
static class ClientJvmCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return getVmName().toLowerCase(Locale.ROOT).contains("client");
public BootstrapCheckResult check(BootstrapContext context) {
if (getVmName().toLowerCase(Locale.ROOT).contains("client")) {
final String message = String.format(
Locale.ROOT,
"JVM is using the client VM [%s] but should be using a server VM for the best performance",
getVmName());
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -476,14 +482,6 @@ final class BootstrapChecks {
return JvmInfo.jvmInfo().getVmName();
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"JVM is using the client VM [%s] but should be using a server VM for the best performance",
getVmName());
}
}
/**
@ -493,8 +491,17 @@ final class BootstrapChecks {
static class UseSerialGCCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return getUseSerialGC().equals("true");
public BootstrapCheckResult check(BootstrapContext context) {
if (getUseSerialGC().equals("true")) {
final String message = String.format(
Locale.ROOT,
"JVM is using the serial collector but should not be for the best performance; " +
"either it's the default for the VM [%s] or -XX:+UseSerialGC was explicitly specified",
JvmInfo.jvmInfo().getVmName());
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -502,15 +509,6 @@ final class BootstrapChecks {
return JvmInfo.jvmInfo().useSerialGC();
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"JVM is using the serial collector but should not be for the best performance; " +
"either it's the default for the VM [%s] or -XX:+UseSerialGC was explicitly specified",
JvmInfo.jvmInfo().getVmName());
}
}
/**
@ -519,8 +517,14 @@ final class BootstrapChecks {
static class SystemCallFilterCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return BootstrapSettings.SYSTEM_CALL_FILTER_SETTING.get(context.settings) && !isSystemCallFilterInstalled();
public BootstrapCheckResult check(BootstrapContext context) {
if (BootstrapSettings.SYSTEM_CALL_FILTER_SETTING.get(context.settings) && !isSystemCallFilterInstalled()) {
final String message = "system call filters failed to install; " +
"check the logs and fix your configuration or disable system call filters at your own risk";
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
// visible for testing
@ -528,21 +532,21 @@ final class BootstrapChecks {
return Natives.isSystemCallFilterInstalled();
}
@Override
public String errorMessage() {
return "system call filters failed to install; " +
"check the logs and fix your configuration or disable system call filters at your own risk";
}
}
abstract static class MightForkCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return isSystemCallFilterInstalled() && mightFork();
public BootstrapCheckResult check(BootstrapContext context) {
if (isSystemCallFilterInstalled() && mightFork()) {
return BootstrapCheckResult.failure(message(context));
} else {
return BootstrapCheckResult.success();
}
}
abstract String message(BootstrapContext context);
// visible for testing
boolean isSystemCallFilterInstalled() {
return Natives.isSystemCallFilterInstalled();
@ -572,7 +576,7 @@ final class BootstrapChecks {
}
@Override
public String errorMessage() {
String message(BootstrapContext context) {
return String.format(
Locale.ROOT,
"OnError [%s] requires forking but is prevented by system call filters ([%s=true]);" +
@ -596,8 +600,7 @@ final class BootstrapChecks {
return JvmInfo.jvmInfo().onOutOfMemoryError();
}
@Override
public String errorMessage() {
String message(BootstrapContext context) {
return String.format(
Locale.ROOT,
"OnOutOfMemoryError [%s] requires forking but is prevented by system call filters ([%s=true]);" +
@ -614,8 +617,17 @@ final class BootstrapChecks {
static class EarlyAccessCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
return "Oracle Corporation".equals(jvmVendor()) && javaVersion().endsWith("-ea");
public BootstrapCheckResult check(BootstrapContext context) {
final String javaVersion = javaVersion();
if ("Oracle Corporation".equals(jvmVendor()) && javaVersion.endsWith("-ea")) {
final String message = String.format(
Locale.ROOT,
"Java version [%s] is an early-access build, only use release builds",
javaVersion);
return BootstrapCheckResult.failure(message);
} else {
return BootstrapCheckResult.success();
}
}
String jvmVendor() {
@ -626,14 +638,6 @@ final class BootstrapChecks {
return Constants.JAVA_VERSION;
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"Java version [%s] is an early-access build, only use release builds",
javaVersion());
}
}
/**
@ -642,7 +646,7 @@ final class BootstrapChecks {
static class G1GCCheck implements BootstrapCheck {
@Override
public boolean check(BootstrapContext context) {
public BootstrapCheckResult check(BootstrapContext context) {
if ("Oracle Corporation".equals(jvmVendor()) && isJava8() && isG1GCEnabled()) {
final String jvmVersion = jvmVersion();
// HotSpot versions on Java 8 match this regular expression; note that this changes with Java 9 after JEP-223
@ -653,10 +657,14 @@ final class BootstrapChecks {
final int major = Integer.parseInt(matcher.group(1));
final int update = Integer.parseInt(matcher.group(2));
// HotSpot versions for Java 8 have major version 25, the bad versions are all versions prior to update 40
return major == 25 && update < 40;
} else {
return false;
if (major == 25 && update < 40) {
final String message = String.format(
Locale.ROOT,
"JVM version [%s] can cause data corruption when used with G1GC; upgrade to at least Java 8u40", jvmVersion);
return BootstrapCheckResult.failure(message);
}
}
return BootstrapCheckResult.success();
}
// visible for testing
@ -682,13 +690,6 @@ final class BootstrapChecks {
return JavaVersion.current().equals(JavaVersion.parse("1.8"));
}
@Override
public String errorMessage() {
return String.format(
Locale.ROOT,
"JVM version [%s] can cause data corruption when used with G1GC; upgrade to at least Java 8u40", jvmVersion());
}
}
}

37
core/src/test/java/org/elasticsearch/bootstrap/BootstrapChecksTests.java
View File

@ -52,6 +52,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class BootstrapChecksTests extends ESTestCase {
private static final BootstrapContext defaultContext = new BootstrapContext(Settings.EMPTY, MetaData.EMPTY_META_DATA);
public void testNonProductionMode() throws NodeValidationException {
@ -126,29 +127,8 @@ public class BootstrapChecksTests extends ESTestCase {
public void testExceptionAggregation() {
final List<BootstrapCheck> checks = Arrays.asList(
new BootstrapCheck() {
@Override
public boolean check(BootstrapContext context) {
return true;
}
@Override
public String errorMessage() {
return "first";
}
},
new BootstrapCheck() {
@Override
public boolean check(BootstrapContext context) {
return true;
}
@Override
public String errorMessage() {
return "second";
}
}
);
context -> BootstrapCheck.BootstrapCheckResult.failure("first"),
context -> BootstrapCheck.BootstrapCheckResult.failure("second"));
final NodeValidationException e =
expectThrows(NodeValidationException.class,
@ -497,7 +477,7 @@ public class BootstrapChecksTests extends ESTestCase {
}
@Override
public String errorMessage() {
String message(BootstrapContext context) {
return "error";
}
};
@ -713,13 +693,8 @@ public class BootstrapChecksTests extends ESTestCase {
public void testAlwaysEnforcedChecks() {
final BootstrapCheck check = new BootstrapCheck() {
@Override
public boolean check(BootstrapContext context) {
return true;
}
@Override
public String errorMessage() {
return "error";
public BootstrapCheckResult check(BootstrapContext context) {
return BootstrapCheckResult.failure("error");
}
@Override

11
core/src/test/java/org/elasticsearch/node/NodeTests.java
View File

@ -65,17 +65,8 @@ public class NodeTests extends ESTestCase {
}
public static class CheckPlugin extends Plugin {
public static final BootstrapCheck CHECK = new BootstrapCheck() {
@Override
public boolean check(BootstrapContext context) {
return false;
}
public static final BootstrapCheck CHECK = context -> BootstrapCheck.BootstrapCheckResult.success();
@Override
public String errorMessage() {
return "boom";
}
};
@Override
public List<BootstrapCheck> getBootstrapChecks() {
return Collections.singletonList(CHECK);

13
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilBootstrapChecksTests.java
View File

@ -59,19 +59,8 @@ public class EvilBootstrapChecksTests extends ESTestCase {
public void testEnforceBootstrapChecks() throws NodeValidationException {
setEsEnforceBootstrapChecks("true");
final List<BootstrapCheck> checks = Collections.singletonList(
new BootstrapCheck() {
@Override
public boolean check(BootstrapContext context) {
return true;
}
final List<BootstrapCheck> checks = Collections.singletonList(context -> BootstrapCheck.BootstrapCheckResult.failure("error"));
@Override
public String errorMessage() {
return "error";
}
}
);
final Logger logger = mock(Logger.class);
final NodeValidationException e = expectThrows(