Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to make sure that log of updating IndexSetting be more detailed #49969

Merged
merged 9 commits into from
Jan 3, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ private void applyChanges(UpdateTask task, ClusterState previousClusterState, Cl
logger.debug("applying settings from cluster state with version {}", newClusterState.version());
final Settings incomingSettings = clusterChangedEvent.state().metaData().settings();
try (Releasable ignored = stopWatch.timing("applying settings")) {
clusterSettings.applySettings(incomingSettings);
clusterSettings.applySettings("cluster", incomingSettings);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,15 @@ public synchronized Settings validateUpdate(Settings settings) {
* @return the unmerged applied settings
*/
public synchronized Settings applySettings(Settings newSettings) {
return applySettings(null, newSettings);
}

/**
* @param target the target to be applied newSettings
* @param newSettings the settings to apply
* @return the unmerged applied settings
*/
public synchronized Settings applySettings(String target, Settings newSettings) {
if (lastSettingsApplied != null && newSettings.equals(lastSettingsApplied)) {
// nothing changed in the settings, ignore
return newSettings;
Expand All @@ -181,7 +190,7 @@ public synchronized Settings applySettings(Settings newSettings) {
List<Runnable> applyRunnables = new ArrayList<>();
for (SettingUpdater<?> settingUpdater : settingUpdaters) {
try {
applyRunnables.add(settingUpdater.updater(current, previous));
applyRunnables.add(settingUpdater.updater(target, current, previous));
} catch (Exception ex) {
logger.warn(() -> new ParameterizedMessage("failed to prepareCommit settings for [{}]", settingUpdater), ex);
throw ex;
Expand Down Expand Up @@ -287,7 +296,7 @@ public Map<String, Tuple<A, B>> getValue(Settings current, Settings previous) {
}

@Override
public void apply(Map<String, Tuple<A, B>> values, Settings current, Settings previous) {
public void apply(String target, Map<String, Tuple<A, B>> values, Settings current, Settings previous) {
for (Map.Entry<String, Tuple<A, B>> entry : values.entrySet()) {
consumer.accept(entry.getKey(), entry.getValue());
}
Expand Down Expand Up @@ -336,7 +345,7 @@ public Map<String, Settings> getValue(Settings current, Settings previous) {
}

@Override
public void apply(Map<String, Settings> values, Settings current, Settings previous) {
public void apply(String target, Map<String, Settings> values, Settings current, Settings previous) {
for (Map.Entry<String, Settings> entry : values.entrySet()) {
consumer.accept(entry.getKey(), entry.getValue());
}
Expand Down Expand Up @@ -584,30 +593,34 @@ public interface SettingUpdater<T> {
/**
* Applies the given value to the updater. This methods will actually run the update.
*/
void apply(T value, Settings current, Settings previous);
void apply(String target, T value, Settings current, Settings previous);

/**
* Updates this updaters value if it has changed.
* @return <code>true</code> iff the value has been updated.
*/
default boolean apply(Settings current, Settings previous) {
return apply(null, current, previous);
}

default boolean apply(String target, Settings current, Settings previous) {
if (hasChanged(current, previous)) {
T value = getValue(current, previous);
apply(value, current, previous);
apply(target, value, current, previous);
return true;
}
return false;
}

/**
* Returns a callable runnable that calls {@link #apply(Object, Settings, Settings)} if the settings
* Returns a callable runnable that calls {@link #apply(String, Object, Settings, Settings)} if the settings
* actually changed. This allows to defer the update to a later point in time while keeping type safety.
* If the value didn't change the returned runnable is a noop.
*/
default Runnable updater(Settings current, Settings previous) {
default Runnable updater(String target, Settings current, Settings previous) {
if (hasChanged(current, previous)) {
T value = getValue(current, previous);
return () -> { apply(value, current, previous);};
return () -> { apply(target, value, current, previous);};
}
return () -> {};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public Settings getValue(Settings current, Settings previous) {
}

@Override
public void apply(Settings value, Settings current, Settings previous) {
public void apply(String target, Settings value, Settings current, Settings previous) {
for (String key : value.keySet()) {
assert loggerPredicate.test(key);
String component = key.substring("logger.".length());
Expand Down
39 changes: 25 additions & 14 deletions server/src/main/java/org/elasticsearch/common/settings/Setting.java
Original file line number Diff line number Diff line change
Expand Up @@ -614,12 +614,12 @@ public Tuple<A, B> getValue(Settings current, Settings previous) {
}

@Override
public void apply(Tuple<A, B> value, Settings current, Settings previous) {
public void apply(String target, Tuple<A, B> value, Settings current, Settings previous) {
if (aSettingUpdater.hasChanged(current, previous)) {
logSettingUpdate(aSetting, current, previous, logger);
logSettingUpdate(target, aSetting, current, previous, logger);
}
if (bSettingUpdater.hasChanged(current, previous)) {
logSettingUpdate(bSetting, current, previous, logger);
logSettingUpdate(target, bSetting, current, previous, logger);
}
consumer.accept(value.v1(), value.v2());
}
Expand Down Expand Up @@ -660,7 +660,7 @@ public Settings getValue(Settings current, Settings previous) {
}

@Override
public void apply(Settings value, Settings current, Settings previous) {
public void apply(String target, Settings value, Settings current, Settings previous) {
consumer.accept(value);
}

Expand Down Expand Up @@ -732,9 +732,10 @@ public Map<AbstractScopedSettings.SettingUpdater<T>, T> getValue(Settings curren
}

@Override
public void apply(Map<AbstractScopedSettings.SettingUpdater<T>, T> value, Settings current, Settings previous) {
public void apply(String target, Map<AbstractScopedSettings.SettingUpdater<T>, T> value,
Settings current, Settings previous) {
for (Map.Entry<AbstractScopedSettings.SettingUpdater<T>, T> entry : value.entrySet()) {
entry.getKey().apply(entry.getValue(), current, previous);
entry.getKey().apply(target, entry.getValue(), current, previous);
}
}
};
Expand Down Expand Up @@ -769,7 +770,7 @@ public Map<String, T> getValue(Settings current, Settings previous) {
}

@Override
public void apply(Map<String, T> value, Settings current, Settings previous) {
public void apply(String target, Map<String, T> value, Settings current, Settings previous) {
consumer.accept(value);
}
};
Expand Down Expand Up @@ -994,8 +995,8 @@ public Settings getValue(Settings current, Settings previous) {
}

@Override
public void apply(Settings value, Settings current, Settings previous) {
Setting.logSettingUpdate(GroupSetting.this, current, previous, logger);
public void apply(String target, Settings value, Settings current, Settings previous) {
Setting.logSettingUpdate(target, GroupSetting.this, current, previous, logger);
consumer.accept(value);
}

Expand Down Expand Up @@ -1052,8 +1053,8 @@ public T getValue(Settings current, Settings previous) {
}

@Override
public void apply(T value, Settings current, Settings previous) {
logSettingUpdate(Setting.this, current, previous, logger);
public void apply(String target, T value, Settings current, Settings previous) {
logSettingUpdate(target, Setting.this, current, previous, logger);
consumer.accept(value);
}
}
Expand Down Expand Up @@ -1463,12 +1464,22 @@ public void diff(Settings.Builder builder, Settings source, Settings defaultSett
}
}

static void logSettingUpdate(Setting setting, Settings current, Settings previous, Logger logger) {
static void logSettingUpdate(String target, Setting setting, Settings current, Settings previous, Logger logger) {
if (logger.isInfoEnabled()) {
if (setting.isFiltered()) {
logger.info("updating [{}]", setting.key);
if (target == null) {
logger.info("updating [{}]", setting.key);
} else {
logger.info("updating [{}] [{}]", target, setting.key);

}
} else {
logger.info("updating [{}] from [{}] to [{}]", setting.key, setting.getRaw(previous), setting.getRaw(current));
if (target == null) {
logger.info("updating [{}] from [{}] to [{}]", setting.key, setting.getRaw(previous), setting.getRaw(current));
} else {
logger.info("updating [{}] [{}] from [{}] to [{}]",
target, setting.key, setting.getRaw(previous), setting.getRaw(current));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ public synchronized boolean updateIndexMetaData(IndexMetaData indexMetaData) {
// nothing to update, same settings
return false;
}
scopedSettings.applySettings(newSettings);
scopedSettings.applySettings(indexMetaData.getIndex().getName(), newSettings);
this.settings = newIndexSettings;
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void testFilteredSettingIsNotLogged() throws Exception {
Settings newSettings = Settings.builder().put("key", "new").build();

Setting<String> filteredSetting = Setting.simpleString("key", Property.Filtered);
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(filteredSetting, newSettings, oldSettings, testLogger),
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate("index1", filteredSetting, newSettings, oldSettings, testLogger),
new MockLogAppender.SeenEventExpectation("secure logging", "org.elasticsearch.test", Level.INFO, "updating [key]"),
new MockLogAppender.UnseenEventExpectation("unwanted old setting name", "org.elasticsearch.test", Level.INFO, "*old*"),
new MockLogAppender.UnseenEventExpectation("unwanted new setting name", "org.elasticsearch.test", Level.INFO, "*new*")
Expand All @@ -126,7 +126,7 @@ public void testRegularSettingUpdateIsFullyLogged() throws Exception {
Settings newSettings = Settings.builder().put("key", "new").build();

Setting<String> regularSetting = Setting.simpleString("key");
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(regularSetting, newSettings, oldSettings, testLogger),
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate("index1", regularSetting, newSettings, oldSettings, testLogger),
new MockLogAppender.SeenEventExpectation("regular logging", "org.elasticsearch.test", Level.INFO,
"updating [key] from [old] to [new]"));
}
Expand Down