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

Add precise logging on unknown or invalid settings

Browse Source 
Today when logging an unknown or invalid setting, the log message does
not contain the source. This means that if we are archiving such a
setting, we do not specify where the setting is from (an index, and
which index, or a persistent or transient cluster setting). This commit
provides such logging for the end user can better understand the
consequences of the unknown or invalid setting.

Relates #20951
This commit is contained in:
Jason Tedor 2016-10-14 23:55:15 -04:00 committed by GitHub
parent f23ae90d92
commit 5a03eb91e6
4 changed files with 88 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java
View File

@ -19,6 +19,8 @@
package org.elasticsearch.cluster.metadata;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.apache.lucene.analysis.Analyzer;
import org.elasticsearch.Version;
import org.elasticsearch.common.component.AbstractComponent;
@ -26,8 +28,8 @@ import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.AnalyzerScope;
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.similarity.SimilarityService;
@ -161,7 +163,10 @@ public class MetaDataIndexUpgradeService extends AbstractComponent {
IndexMetaData archiveBrokenIndexSettings(IndexMetaData indexMetaData) {
final Settings settings = indexMetaData.getSettings();
final Settings upgrade = indexScopedSettings.archiveUnknownOrBrokenSettings(settings);
final Settings upgrade = indexScopedSettings.archiveUnknownOrInvalidSettings(
settings,
e -> logger.warn("{} ignoring unknown index setting: [{}] with value [{}]; archiving", indexMetaData.getIndex(), e.getKey(), e.getValue()),
(e, ex) -> logger.warn((Supplier<?>) () -> new ParameterizedMessage("{} ignoring invalid index setting: [{}] with value [{}]; archiving", indexMetaData.getIndex(), e.getKey(), e.getValue()), ex));
if (upgrade != settings) {
return IndexMetaData.builder(indexMetaData).settings(upgrade).build();
} else {

32
core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java
View File

@ -498,11 +498,21 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
}
/**
* Archives broken or unknown settings. Any setting that is not recognized or fails
* validation will be archived. This means the setting is prefixed with {@value ARCHIVED_SETTINGS_PREFIX}
* and remains in the settings object. This can be used to detect broken settings via APIs.
* Archives invalid or unknown settings. Any setting that is not recognized or fails validation
* will be archived. This means the setting is prefixed with {@value ARCHIVED_SETTINGS_PREFIX}
* and remains in the settings object. This can be used to detect invalid settings via APIs.
*
* @param settings the {@link Settings} instance to scan for unknown or invalid settings
* @param unknownConsumer callback on unknown settings (consumer receives unknown key and its
* associated value)
* @param invalidConsumer callback on invalid settings (consumer receives invalid key, its
* associated value and an exception)
* @return a {@link Settings} instance with the unknown or invalid settings archived
*/
public Settings archiveUnknownOrBrokenSettings(Settings settings) {
public Settings archiveUnknownOrInvalidSettings(
final Settings settings,
final Consumer<Map.Entry<String, String>> unknownConsumer,
final BiConsumer<Map.Entry<String, String>, IllegalArgumentException> invalidConsumer) {
Settings.Builder builder = Settings.builder();
boolean changed = false;
for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
@ -516,10 +526,10 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
builder.put(entry.getKey(), entry.getValue());
} else {
changed = true;
logger.warn("found unknown setting: {} value: {} - archiving", entry.getKey(), entry.getValue());
unknownConsumer.accept(entry);
/*
* We put them back in here such that tools can check from the outside if there are any indices with broken
* settings. The setting can remain there but we want users to be aware that some of their setting are broken and
* We put them back in here such that tools can check from the outside if there are any indices with invalid
* settings. The setting can remain there but we want users to be aware that some of their setting are invalid and
* they can research why and what they need to do to replace them.
*/
builder.put(ARCHIVED_SETTINGS_PREFIX + entry.getKey(), entry.getValue());
@ -527,12 +537,10 @@ public abstract class AbstractScopedSettings extends AbstractComponent {
}
} catch (IllegalArgumentException ex) {
changed = true;
logger.warn(
(Supplier<?>) () -> new ParameterizedMessage(
"found invalid setting: {} value: {} - archiving", entry.getKey(), entry.getValue()), ex);
invalidConsumer.accept(entry, ex);
/*
* We put them back in here such that tools can check from the outside if there are any indices with broken settings. The
* setting can remain there but we want users to be aware that some of their setting are broken and they can research why
* We put them back in here such that tools can check from the outside if there are any indices with invalid settings. The
* setting can remain there but we want users to be aware that some of their setting are invalid and they can research why
* and what they need to do to replace them.
*/
builder.put(ARCHIVED_SETTINGS_PREFIX + entry.getKey(), entry.getValue());

28
core/src/main/java/org/elasticsearch/gateway/Gateway.java
View File

@ -21,7 +21,6 @@ package org.elasticsearch.gateway;
import com.carrotsearch.hppc.ObjectFloatHashMap;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.action.FailedNodeException;
import org.elasticsearch.cluster.ClusterChangedEvent;
@ -38,6 +37,7 @@ import org.elasticsearch.index.Index;
import org.elasticsearch.indices.IndicesService;
import java.util.Arrays;
import java.util.Map;
import java.util.function.Supplier;
public class Gateway extends AbstractComponent implements ClusterStateListener {
@ -146,13 +146,35 @@ public class Gateway extends AbstractComponent implements ClusterStateListener {
}
}
final ClusterSettings clusterSettings = clusterService.getClusterSettings();
metaDataBuilder.persistentSettings(clusterSettings.archiveUnknownOrBrokenSettings(metaDataBuilder.persistentSettings()));
metaDataBuilder.transientSettings(clusterSettings.archiveUnknownOrBrokenSettings(metaDataBuilder.transientSettings()));
metaDataBuilder.persistentSettings(
clusterSettings.archiveUnknownOrInvalidSettings(
metaDataBuilder.persistentSettings(),
e -> logUnknownSetting("persistent", e),
(e, ex) -> logInvalidSetting("persistent", e, ex)));
metaDataBuilder.transientSettings(
clusterSettings.archiveUnknownOrInvalidSettings(
metaDataBuilder.transientSettings(),
e -> logUnknownSetting("transient", e),
(e, ex) -> logInvalidSetting("transient", e, ex)));
ClusterState.Builder builder = ClusterState.builder(clusterService.getClusterName());
builder.metaData(metaDataBuilder);
listener.onSuccess(builder.build());
}
private void logUnknownSetting(String settingType, Map.Entry<String, String> e) {
logger.warn("ignoring unknown {} setting: [{}] with value [{}]; archiving", settingType, e.getKey(), e.getValue());
}
private void logInvalidSetting(String settingType, Map.Entry<String, String> e, IllegalArgumentException ex) {
logger.warn(
(org.apache.logging.log4j.util.Supplier<?>)
() -> new ParameterizedMessage("ignoring invalid {} setting: [{}] with value [{}]; archiving",
settingType,
e.getKey(),
e.getValue()),
ex);
}
@Override
public void clusterChanged(final ClusterChangedEvent event) {
// order is important, first metaState, and then shardsState

48
core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java
View File

@ -16,11 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.index;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaDataIndexUpgradeService;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Setting;
@ -29,18 +29,20 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.translog.Translog;
import org.elasticsearch.indices.mapper.MapperRegistry;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.VersionUtils;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.core.StringContains.containsString;
import static org.hamcrest.object.HasToString.hasToString;
public class IndexSettingsTests extends ESTestCase {
public void testRunListener() {
@ -348,26 +350,48 @@ public class IndexSettingsTests extends ESTestCase {
assertEquals(actualNewTranslogFlushThresholdSize, settings.getFlushThresholdSize());
}
public void testArchiveBrokenIndexSettings() {
Settings settings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrBrokenSettings(Settings.EMPTY);
Settings settings =
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrInvalidSettings(
Settings.EMPTY,
e -> { assert false : "should not have been invoked, no unknown settings"; },
(e, ex) -> { assert false : "should not have been invoked, no invalid settings"; });
assertSame(settings, Settings.EMPTY);
settings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrBrokenSettings(Settings.builder()
.put("index.refresh_interval", "-200").build());
settings =
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrInvalidSettings(
Settings.builder().put("index.refresh_interval", "-200").build(),
e -> { assert false : "should not have been invoked, no invalid settings"; },
(e, ex) -> {
assertThat(e.getKey(), equalTo("index.refresh_interval"));
assertThat(e.getValue(), equalTo("-200"));
assertThat(ex, hasToString(containsString("failed to parse setting [index.refresh_interval] with value [-200]")));
});
assertEquals("-200", settings.get("archived.index.refresh_interval"));
assertNull(settings.get("index.refresh_interval"));
Settings prevSettings = settings; // no double archive
settings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrBrokenSettings(prevSettings);
settings =
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrInvalidSettings(
prevSettings,
e -> { assert false : "should not have been invoked, no unknown settings"; },
(e, ex) -> { assert false : "should not have been invoked, no invalid settings"; });
assertSame(prevSettings, settings);
settings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrBrokenSettings(Settings.builder()
.put("index.version.created", Version.CURRENT.id) // private setting
.put("index.unknown", "foo")
.put("index.refresh_interval", "2s").build());
settings =
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrInvalidSettings(
Settings.builder()
.put("index.version.created", Version.CURRENT.id) // private setting
.put("index.unknown", "foo")
.put("index.refresh_interval", "2s").build(),
e -> {
assertThat(e.getKey(), equalTo("index.unknown"));
assertThat(e.getValue(), equalTo("foo"));
},
(e, ex) -> { assert false : "should not have been invoked, no invalid settings"; });
assertEquals("foo", settings.get("archived.index.unknown"));
assertEquals(Integer.toString(Version.CURRENT.id), settings.get("index.version.created"));
assertEquals("2s", settings.get("index.refresh_interval"));
}
}