honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Settings: Introduce settings updater for a list of settings (#28338) 

This introduces a settings updater that allows to specify a list of
settings. Whenever one of those settings changes, the whole block of
settings is passed to the consumer.

This also fixes an issue with affix settings, when used in combination
with group settings, which could result in no found settings when used
to get a setting for a namespace.

Lastly logging has been slightly changed, so that filtered settings now
only log the setting key.

Another bug has been fixed for the mock log appender, which did not
work, when checking for the exact message.

Closes #28047
This commit is contained in:
Alexander Reelsen 2018-01-24 09:47:17 +01:00 committed by GitHub
parent b10d166190
commit a87714aafc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 187 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java
View File

@ -194,6 +194,16 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
addSettingsUpdater(setting.newUpdater(consumer, logger, validator));
}
/**
* Adds a settings consumer that is only executed if any setting in the supplied list of settings is changed. In that case all the
* settings are specified in the argument are returned.
*
* Also automatically adds empty consumers for all settings in order to activate logging
*/
public synchronized void addSettingsUpdateConsumer(Consumer<Settings> consumer, List<? extends Setting<?>> settings) {
addSettingsUpdater(Setting.groupedSettingsUpdater(consumer, logger, settings));
}
/**
* Adds a settings consumer for affix settings. Affix settings have a namespace associated to it that needs to be available to the
* consumer in order to be processed correctly.

66
server/src/main/java/org/elasticsearch/common/settings/Setting.java
View File

@ -509,10 +509,10 @@ public class Setting<T> implements ToXContentObject {
@Override
public void apply(Tuple<A, B> value, Settings current, Settings previous) {
if (aSettingUpdater.hasChanged(current, previous)) {
logger.info("updating [{}] from [{}] to [{}]", aSetting.key, aSetting.getRaw(previous), aSetting.getRaw(current));
logSettingUpdate(aSetting, current, previous, logger);
}
if (bSettingUpdater.hasChanged(current, previous)) {
logger.info("updating [{}] from [{}] to [{}]", bSetting.key, bSetting.getRaw(previous), bSetting.getRaw(current));
logSettingUpdate(bSetting, current, previous, logger);
}
consumer.accept(value.v1(), value.v2());
}
@ -524,6 +524,46 @@ public class Setting<T> implements ToXContentObject {
};
}
static AbstractScopedSettings.SettingUpdater<Settings> groupedSettingsUpdater(Consumer<Settings> consumer, Logger logger,
final List<? extends Setting<?>> configuredSettings) {
return new AbstractScopedSettings.SettingUpdater<Settings>() {
private Settings get(Settings settings) {
return settings.filter(s -> {
for (Setting<?> setting : configuredSettings) {
if (setting.key.match(s)) {
return true;
}
}
return false;
});
}
@Override
public boolean hasChanged(Settings current, Settings previous) {
Settings currentSettings = get(current);
Settings previousSettings = get(previous);
return currentSettings.equals(previousSettings) == false;
}
@Override
public Settings getValue(Settings current, Settings previous) {
return get(current);
}
@Override
public void apply(Settings value, Settings current, Settings previous) {
consumer.accept(value);
}
@Override
public String toString() {
return "Updater grouped: " + configuredSettings.stream().map(Setting::getKey).collect(Collectors.joining(", "));
}
};
}
public static class AffixSetting<T> extends Setting<T> {
private final AffixKey key;
private final Function<String, Setting<T>> delegateFactory;
@ -541,7 +581,7 @@ public class Setting<T> implements ToXContentObject {
}
private Stream<String> matchStream(Settings settings) {
return settings.keySet().stream().filter((key) -> match(key)).map(settingKey -> key.getConcreteString(settingKey));
return settings.keySet().stream().filter(this::match).map(key::getConcreteString);
}
public Set<String> getSettingsDependencies(String settingsKey) {
@ -812,9 +852,7 @@ public class Setting<T> implements ToXContentObject {
@Override
public void apply(Settings value, Settings current, Settings previous) {
if (logger.isInfoEnabled()) { // getRaw can create quite some objects
logger.info("updating [{}] from [{}] to [{}]", key, getRaw(previous), getRaw(current));
}
Setting.logSettingUpdate(GroupSetting.this, current, previous, logger);
consumer.accept(value);
}
@ -902,7 +940,7 @@ public class Setting<T> implements ToXContentObject {
@Override
public void apply(T value, Settings current, Settings previous) {
logger.info("updating [{}] from [{}] to [{}]", key, getRaw(previous), getRaw(current));
logSettingUpdate(Setting.this, current, previous, logger);
consumer.accept(value);
}
}
@ -1138,6 +1176,16 @@ public class Setting<T> implements ToXContentObject {
}
}
static void logSettingUpdate(Setting setting, Settings current, Settings previous, Logger logger) {
if (logger.isInfoEnabled()) {
if (setting.isFiltered()) {
logger.info("updating [{}]", setting.key);
} else {
logger.info("updating [{}] from [{}] to [{}]", setting.key, setting.getRaw(previous), setting.getRaw(current));
}
}
}
public static Setting<Settings> groupSetting(String key, Property... properties) {
return groupSetting(key, (s) -> {}, properties);
}
@ -1308,8 +1356,8 @@ public class Setting<T> implements ToXContentObject {
if (suffix == null) {
pattern = Pattern.compile("(" + Pattern.quote(prefix) + "((?:[-\\w]+[.])*[-\\w]+$))");
} else {
// the last part of this regexp is for lists since they are represented as x.${namespace}.y.1, x.${namespace}.y.2
pattern = Pattern.compile("(" + Pattern.quote(prefix) + "([-\\w]+)\\." + Pattern.quote(suffix) + ")(?:\\.\\d+)?");
// the last part of this regexp is to support both list and group keys
pattern = Pattern.compile("(" + Pattern.quote(prefix) + "([-\\w]+)\\." + Pattern.quote(suffix) + ")(?:\\..*)?");
}
}

76
server/src/test/java/org/elasticsearch/common/settings/SettingTests.java
View File

@ -38,6 +38,7 @@ import java.util.stream.Stream;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
@ -712,4 +713,79 @@ public class SettingTests extends ESTestCase {
assertThat(setting.get(Settings.EMPTY).getMillis(), equalTo(random.getMillis() * factor));
}
public void testSettingsGroupUpdater() {
Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
Setting<Integer> intSetting2 = Setting.intSetting("prefix.same", 1, Property.NodeScope, Property.Dynamic);
AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
Arrays.asList(intSetting, intSetting2));
Settings current = Settings.builder().put("prefix.foo", 123).put("prefix.same", 5555).build();
Settings previous = Settings.builder().put("prefix.foo", 321).put("prefix.same", 5555).build();
assertTrue(updater.apply(current, previous));
}
public void testSettingsGroupUpdaterRemoval() {
Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
Setting<Integer> intSetting2 = Setting.intSetting("prefix.same", 1, Property.NodeScope, Property.Dynamic);
AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
Arrays.asList(intSetting, intSetting2));
Settings current = Settings.builder().put("prefix.same", 5555).build();
Settings previous = Settings.builder().put("prefix.foo", 321).put("prefix.same", 5555).build();
assertTrue(updater.apply(current, previous));
}
public void testSettingsGroupUpdaterWithAffixSetting() {
Setting<Integer> intSetting = Setting.intSetting("prefix.foo", 1, Property.NodeScope, Property.Dynamic);
Setting.AffixSetting<String> prefixKeySetting =
Setting.prefixKeySetting("prefix.foo.bar.", key -> Setting.simpleString(key, Property.NodeScope, Property.Dynamic));
Setting.AffixSetting<String> affixSetting =
Setting.affixKeySetting("prefix.foo.", "suffix", key -> Setting.simpleString(key,Property.NodeScope, Property.Dynamic));
AbstractScopedSettings.SettingUpdater<Settings> updater = Setting.groupedSettingsUpdater(s -> {}, logger,
Arrays.asList(intSetting, prefixKeySetting, affixSetting));
Settings.Builder currentSettingsBuilder = Settings.builder()
.put("prefix.foo.bar.baz", "foo")
.put("prefix.foo.infix.suffix", "foo");
Settings.Builder previousSettingsBuilder = Settings.builder()
.put("prefix.foo.bar.baz", "foo")
.put("prefix.foo.infix.suffix", "foo");
boolean removePrefixKeySetting = randomBoolean();
boolean changePrefixKeySetting = randomBoolean();
boolean removeAffixKeySetting = randomBoolean();
boolean changeAffixKeySetting = randomBoolean();
boolean removeAffixNamespace = randomBoolean();
if (removePrefixKeySetting) {
previousSettingsBuilder.remove("prefix.foo.bar.baz");
}
if (changePrefixKeySetting) {
currentSettingsBuilder.put("prefix.foo.bar.baz", "bar");
}
if (removeAffixKeySetting) {
previousSettingsBuilder.remove("prefix.foo.infix.suffix");
}
if (changeAffixKeySetting) {
currentSettingsBuilder.put("prefix.foo.infix.suffix", "bar");
}
if (removeAffixKeySetting == false && changeAffixKeySetting == false && removeAffixNamespace) {
currentSettingsBuilder.remove("prefix.foo.infix.suffix");
currentSettingsBuilder.put("prefix.foo.infix2.suffix", "bar");
previousSettingsBuilder.put("prefix.foo.infix2.suffix", "bar");
}
boolean expectedChange = removeAffixKeySetting || removePrefixKeySetting || changeAffixKeySetting || changePrefixKeySetting
|| removeAffixNamespace;
assertThat(updater.apply(currentSettingsBuilder.build(), previousSettingsBuilder.build()), is(expectedChange));
}
public void testAffixNamespacesWithGroupSetting() {
final Setting.AffixSetting<Settings> affixSetting =
Setting.affixKeySetting("prefix.","suffix",
(key) -> Setting.groupSetting(key + ".", Setting.Property.Dynamic, Setting.Property.NodeScope));
assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix", "anything").build()), hasSize(1));
assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix.anything", "anything").build()), hasSize(1));
}
}

44
server/src/test/java/org/elasticsearch/common/settings/SettingsFilterTests.java
View File

@ -18,16 +18,22 @@
*/
package org.elasticsearch.common.settings;
import org.elasticsearch.common.Strings;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.logging.ServerLoggers;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLogAppender;
import org.elasticsearch.test.rest.FakeRestRequest;
import java.io.IOException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.function.Consumer;
import static org.hamcrest.CoreMatchers.equalTo;
@ -100,7 +106,43 @@ public class SettingsFilterTests extends ESTestCase {
.build(),
"a.b.*.d"
);
}
public void testFilteredSettingIsNotLogged() throws Exception {
Settings oldSettings = Settings.builder().put("key", "old").build();
Settings newSettings = Settings.builder().put("key", "new").build();
Setting<String> filteredSetting = Setting.simpleString("key", Property.Filtered);
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(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*")
);
}
public void testRegularSettingUpdateIsFullyLogged() throws Exception {
Settings oldSettings = Settings.builder().put("key", "old").build();
Settings newSettings = Settings.builder().put("key", "new").build();
Setting<String> regularSetting = Setting.simpleString("key");
assertExpectedLogMessages((testLogger) -> Setting.logSettingUpdate(regularSetting, newSettings, oldSettings, testLogger),
new MockLogAppender.SeenEventExpectation("regular logging", "org.elasticsearch.test", Level.INFO,
"updating [key] from [old] to [new]"));
}
private void assertExpectedLogMessages(Consumer<Logger> consumer,
MockLogAppender.LoggingExpectation ... expectations) throws IllegalAccessException {
Logger testLogger = Loggers.getLogger("org.elasticsearch.test");
MockLogAppender appender = new MockLogAppender();
ServerLoggers.addAppender(testLogger, appender);
try {
appender.start();
Arrays.stream(expectations).forEach(appender::addExpectation);
consumer.accept(testLogger);
appender.assertAllExpectationsMatched();
} finally {
ServerLoggers.removeAppender(testLogger, appender);
}
}
private void testFiltering(Settings source, Settings filtered, String... patterns) throws IOException {

2
test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java
View File

@ -92,7 +92,7 @@ public class MockLogAppender extends AbstractAppender {
saw = true;
}
} else {
if (event.getMessage().toString().contains(message)) {
if (event.getMessage().getFormattedMessage().contains(message)) {
saw = true;
}
}