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
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public abstract class AbstractScopedSettings {
private static final Pattern GROUP_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+$");
private static final Pattern AFFIX_KEY_PATTERN = Pattern.compile("^(?:[-\\w]+[.])+[*](?:[.][-\\w]+)+$");

protected final Logger logger = LogManager.getLogger(this.getClass());
private final Logger logger;

private final Settings settings;
private final List<SettingUpdater<?>> settingUpdaters = new CopyOnWriteArrayList<>();
Expand All @@ -70,6 +70,7 @@ protected AbstractScopedSettings(
final Set<Setting<?>> settingsSet,
final Set<SettingUpgrader<?>> settingUpgraders,
final Setting.Property scope) {
this.logger = LogManager.getLogger(this.getClass());
this.settings = settings;
this.lastSettingsApplied = Settings.EMPTY;

Expand Down Expand Up @@ -110,7 +111,8 @@ protected void validateSettingKey(Setting<?> setting) {
}
}

protected AbstractScopedSettings(Settings nodeSettings, Settings scopeSettings, AbstractScopedSettings other) {
protected AbstractScopedSettings(Settings nodeSettings, Settings scopeSettings, AbstractScopedSettings other, Logger logger) {
this.logger = logger;
this.settings = nodeSettings;
this.lastSettingsApplied = scopeSettings;
this.scope = other.scope;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider;
import org.elasticsearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettings;
Expand Down Expand Up @@ -186,7 +187,7 @@ public IndexScopedSettings(Settings settings, Set<Setting<?>> settingsSet) {
}

private IndexScopedSettings(Settings settings, IndexScopedSettings other, IndexMetaData metaData) {
super(settings, metaData.getSettings(), other);
super(settings, metaData.getSettings(), other, Loggers.getLogger(IndexScopedSettings.class, metaData.getIndex()));
}

public IndexScopedSettings copy(Settings settings, IndexMetaData metaData) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,26 @@
*/
package org.elasticsearch.common.settings;

import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.appender.AbstractAppender;
import org.apache.logging.log4j.core.filter.RegexFilter;
import org.apache.logging.log4j.core.impl.MementoMessage;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.AbstractScopedSettings.SettingUpdater;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLogAppender;
import org.elasticsearch.test.junit.annotations.TestLogging;

import java.util.Arrays;
import java.util.Collections;
Expand All @@ -40,6 +52,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.index.IndexSettingsTests.newIndexMeta;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
Expand All @@ -49,6 +62,27 @@

public class SettingTests extends ESTestCase {

public static class MockAppender extends AbstractAppender {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is now unused

private LogEvent lastEvent;

public MockAppender(final String name) throws IllegalAccessException {
super(name, RegexFilter.createFilter(".*(\n.*)*", new String[0], false, null, null), null);
}

@Override
public void append(LogEvent event) {
lastEvent = event.toImmutable();
}

public MementoMessage lastMementoMessage() {
return (MementoMessage) lastEvent.getMessage();
}

public String getLoggerMaker() {
return lastEvent.getMarker().getName();
}
}

public void testGet() {
Setting<Boolean> booleanSetting = Setting.boolSetting("foo.bar", false, Property.Dynamic, Property.NodeScope);
assertFalse(booleanSetting.get(Settings.EMPTY));
Expand Down Expand Up @@ -1092,4 +1126,35 @@ public void testNonSecureSettingInKeystore() {
assertThat(e.getMessage(), containsString("must be stored inside elasticsearch.yml"));
}

@TestLogging(value="org.elasticsearch.common.settings.IndexScopedSettings:INFO",
reason="to ensure we log INFO-level messages from IndexScopedSettings")
public void testLogSettingUpdate() throws Exception {
final IndexMetaData metaData = newIndexMeta("index1",
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "20s").build());
final IndexSettings settings = new IndexSettings(metaData, Settings.EMPTY);

final MockLogAppender mockLogAppender = new MockLogAppender();
mockLogAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
"message",
"org.elasticsearch.common.settings.IndexScopedSettings",
Level.INFO,
"updating [index.refresh_interval] from [20s] to [10s]") {
@Override
public boolean innerMatch(LogEvent event) {
return event.getMarker().getName().equals(" [index1]");
}
});
mockLogAppender.start();
final Logger logger = LogManager.getLogger(IndexScopedSettings.class);
try {
Loggers.addAppender(logger, mockLogAppender);
settings.updateIndexMetaData(newIndexMeta("index1",
Settings.builder().put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s").build()));

mockLogAppender.assertAllExpectationsMatched();
} finally {
Loggers.removeAppender(logger, mockLogAppender);
mockLogAppender.stop();
}
}
}