Skip to content

Commit

Permalink
Do not lock on reads of XPackLicenseState (#52437)
Browse files Browse the repository at this point in the history 
XPackLicenseState reads to necessary to validate a number of cluster
operations. This reads occasionally occur on transport threads which
should not be blocked. Currently we sychronize when reading. However,
this is unecessary as only a single piece of state is updateable. This
commit makes this state volatile and removes the locking.
  • Loading branch information
@Tim-Brooks
Tim-Brooks committed Feb 25, 2020
1 parent a891180 commit 48ee863
Showing 1 changed file with 79 additions and 61 deletions.
140 changes: 79 additions & 61 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.Objects;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Predicate;

/**
* A holder for the current state of the license for all xpack features.
Expand Down Expand Up @@ -321,25 +323,41 @@ private static class Status {
private final boolean isSecurityEnabled;
private final boolean isSecurityExplicitlyEnabled;

private Status status = new Status(OperationMode.TRIAL, true);
// Since Status is the only field that can be updated, we do not need to synchronize access to
// XPackLicenseState. However, if status is read multiple times in a method, it can change in between
// reads. Methods should use `executeAgainstStatus` and `checkAgainstStatus` to ensure that the status
// is only read once.
private volatile Status status = new Status(OperationMode.TRIAL, true);

public XPackLicenseState(Settings settings) {
this.listeners = new CopyOnWriteArrayList<>();
this.isSecurityEnabled = XPackSettings.SECURITY_ENABLED.get(settings);
this.isSecurityExplicitlyEnabled = isSecurityEnabled && isSecurityExplicitlyEnabled(settings);
}

private XPackLicenseState(XPackLicenseState xPackLicenseState) {
this.listeners = xPackLicenseState.listeners;
this.isSecurityEnabled = xPackLicenseState.isSecurityEnabled;
this.isSecurityExplicitlyEnabled = xPackLicenseState.isSecurityExplicitlyEnabled;
this.status = xPackLicenseState.status;
private XPackLicenseState(List<LicenseStateListener> listeners, boolean isSecurityEnabled, boolean isSecurityExplicitlyEnabled,
Status status) {

this.listeners = listeners;
this.isSecurityEnabled = isSecurityEnabled;
this.isSecurityExplicitlyEnabled = isSecurityExplicitlyEnabled;
this.status = status;
}

private static boolean isSecurityExplicitlyEnabled(Settings settings) {
return settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey());
}

/** Performs function against status, only reading the status once to avoid races */
private <T> T executeAgainstStatus(Function<Status, T> statusFn) {
return statusFn.apply(this.status);
}

/** Performs predicate against status, only reading the status once to avoid races */
private boolean checkAgainstStatus(Predicate<Status> statusPredicate) {
return statusPredicate.test(this.status);
}

/**
* Updates the current state of the license, which will change what features are available.
*
Expand All @@ -350,9 +368,7 @@ private static boolean isSecurityExplicitlyEnabled(Settings settings) {
* trial was prior to this metadata being tracked (6.1)
*/
void update(OperationMode mode, boolean active, @Nullable Version mostRecentTrialVersion) {
synchronized (this) {
status = new Status(mode, active);
}
status = new Status(mode, active);
listeners.forEach(LicenseStateListener::licenseStateChanged);
}

Expand All @@ -367,13 +383,13 @@ public void removeListener(final LicenseStateListener listener) {
}

/** Return the current license type. */
public synchronized OperationMode getOperationMode() {
return status.mode;
public OperationMode getOperationMode() {
return executeAgainstStatus(status -> status.mode);
}

/** Return true if the license is currently within its time boundaries, false otherwise. */
public synchronized boolean isActive() {
return status.active;
public boolean isActive() {
return checkAgainstStatus(status -> status.active);
}

/**
Expand Down Expand Up @@ -435,26 +451,27 @@ public enum AllowedRealmType {
/**
* @return the type of realms that are enabled based on the license {@link OperationMode}
*/
public synchronized AllowedRealmType allowedRealmType() {
final boolean isSecurityCurrentlyEnabled =
isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled);
if (isSecurityCurrentlyEnabled) {
switch (status.mode) {
case PLATINUM:
case ENTERPRISE:
case TRIAL:
return AllowedRealmType.ALL;
case GOLD:
return AllowedRealmType.DEFAULT;
case BASIC:
case STANDARD:
return AllowedRealmType.NATIVE;
default:
return AllowedRealmType.NONE;
public AllowedRealmType allowedRealmType() {
return executeAgainstStatus(status -> {
final boolean isSecurityCurrentlyEnabled = isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled);
if (isSecurityCurrentlyEnabled) {
switch (status.mode) {
case PLATINUM:
case ENTERPRISE:
case TRIAL:
return AllowedRealmType.ALL;
case GOLD:
return AllowedRealmType.DEFAULT;
case BASIC:
case STANDARD:
return AllowedRealmType.NATIVE;
default:
return AllowedRealmType.NONE;
}
} else {
return AllowedRealmType.NONE;
}
} else {
return AllowedRealmType.NONE;
}
});
}

/**
Expand Down Expand Up @@ -674,8 +691,8 @@ public boolean isEnrichAllowed() {
* <p>
* EQL is available for all license types except {@link OperationMode#MISSING}
*/
public synchronized boolean isEqlAllowed() {
return status.active;
public boolean isEqlAllowed() {
return checkAgainstStatus(status -> status.active);
}

/**
Expand Down Expand Up @@ -736,17 +753,15 @@ public boolean isAnalyticsAllowed() {
return isActive();
}

public synchronized boolean isTrialLicense() {
return status.mode == OperationMode.TRIAL;
}

/**
* @return true if security is available to be used with the current license type
*/
public synchronized boolean isSecurityAvailable() {
OperationMode mode = status.mode;
return mode == OperationMode.GOLD || mode == OperationMode.PLATINUM || mode == OperationMode.STANDARD ||
public boolean isSecurityAvailable() {
return checkAgainstStatus(status -> {
OperationMode mode = status.mode;
return mode == OperationMode.GOLD || mode == OperationMode.PLATINUM || mode == OperationMode.STANDARD ||
mode == OperationMode.TRIAL || mode == OperationMode.BASIC || mode == OperationMode.ENTERPRISE;
});
}

/**
Expand All @@ -757,13 +772,15 @@ public synchronized boolean isSecurityAvailable() {
* <li>xpack.security.enabled not specified as a setting</li>
* </ul>
*/
public synchronized boolean isSecurityDisabledByLicenseDefaults() {
switch (status.mode) {
case TRIAL:
case BASIC:
return isSecurityEnabled && isSecurityExplicitlyEnabled == false;
}
return false;
public boolean isSecurityDisabledByLicenseDefaults() {
return checkAgainstStatus(status -> {
switch (status.mode) {
case TRIAL:
case BASIC:
return isSecurityEnabled && isSecurityExplicitlyEnabled == false;
}
return false;
});
}

public static boolean isTransportTlsRequired(License license, Settings settings) {
Expand Down Expand Up @@ -831,12 +848,12 @@ public static boolean isAllowedByOperationMode(
* lived but instead used within a method when a consistent view of the license state
* is needed for multiple interactions with the license state.
*/
public synchronized XPackLicenseState copyCurrentLicenseState() {
return new XPackLicenseState(this);
public XPackLicenseState copyCurrentLicenseState() {
return executeAgainstStatus(status -> new XPackLicenseState(listeners, isSecurityEnabled, isSecurityExplicitlyEnabled, status));
}

private synchronized boolean isAllowedBySecurity() {
return isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled);
private boolean isAllowedBySecurity() {
return checkAgainstStatus(status -> isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled));
}

/**
Expand All @@ -849,16 +866,17 @@ private synchronized boolean isAllowedBySecurity() {
*
* @return true if feature is allowed, otherwise false
*/
private synchronized boolean isAllowedByLicenseAndSecurity(
private boolean isAllowedByLicenseAndSecurity(
OperationMode minimumMode, boolean needSecurity, boolean needActive, boolean allowTrial) {

if (needSecurity && false == isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled)) {
return false;
}
if (needActive && false == status.active) {
return false;
}
return isAllowedByOperationMode(status.mode, minimumMode, allowTrial);
return checkAgainstStatus(status -> {
if (needSecurity && false == isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled)) {
return false;
}
if (needActive && false == status.active) {
return false;
}
return isAllowedByOperationMode(status.mode, minimumMode, allowTrial);
});
}

}

0 comments on commit 48ee863

Please sign in to comment.