Emit settings deprecation logging at most once
When a setting is deprecated, if that setting is used repeatedly we currently emit a deprecation warning every time the setting is used. In cases like hitting settings endpoints over and over against a node with a lot of deprecated settings, this can lead to excessive deprecation warnings which can crush a node. This commit ensures that a given setting only sees deprecation logging at most once. Relates #25457
This commit is contained in:
parent
9714c77c84
commit
da59c178e2
|
@ -345,7 +345,7 @@ public class Setting<T> extends ToXContentToBytes {
|
||||||
/** Logs a deprecation warning if the setting is deprecated and used. */
|
/** Logs a deprecation warning if the setting is deprecated and used. */
|
||||||
protected void checkDeprecation(Settings settings) {
|
protected void checkDeprecation(Settings settings) {
|
||||||
// They're using the setting, so we need to tell them to stop
|
// They're using the setting, so we need to tell them to stop
|
||||||
if (this.isDeprecated() && this.exists(settings)) {
|
if (this.isDeprecated() && this.exists(settings) && settings.addDeprecatedSetting(this)) {
|
||||||
// It would be convenient to show its replacement key, but replacement is often not so simple
|
// It would be convenient to show its replacement key, but replacement is often not so simple
|
||||||
final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(getClass()));
|
final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(getClass()));
|
||||||
deprecationLogger.deprecated("[{}] setting was deprecated in Elasticsearch and will be removed in a future release! " +
|
deprecationLogger.deprecated("[{}] setting was deprecated in Elasticsearch and will be removed in a future release! " +
|
||||||
|
|
|
@ -55,7 +55,6 @@ import java.util.Dictionary;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.Iterator;
|
import java.util.Iterator;
|
||||||
import java.util.LinkedHashMap;
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Locale;
|
import java.util.Locale;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
@ -63,6 +62,7 @@ import java.util.NoSuchElementException;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.TreeMap;
|
import java.util.TreeMap;
|
||||||
|
import java.util.concurrent.ConcurrentHashMap;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
import java.util.function.Predicate;
|
import java.util.function.Predicate;
|
||||||
|
@ -93,6 +93,22 @@ public final class Settings implements ToXContent {
|
||||||
/** The first level of setting names. This is constructed lazily in {@link #names()}. */
|
/** The first level of setting names. This is constructed lazily in {@link #names()}. */
|
||||||
private final SetOnce<Set<String>> firstLevelNames = new SetOnce<>();
|
private final SetOnce<Set<String>> firstLevelNames = new SetOnce<>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The set of deprecated settings tracked by this settings object.
|
||||||
|
*/
|
||||||
|
private final Set<String> deprecatedSettings = Collections.newSetFromMap(new ConcurrentHashMap<>());
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Add the setting as a tracked deprecated setting.
|
||||||
|
*
|
||||||
|
* @param setting the deprecated setting to track
|
||||||
|
* @return true if the setting was not already tracked as a deprecated setting, otherwise false
|
||||||
|
*/
|
||||||
|
boolean addDeprecatedSetting(final Setting setting) {
|
||||||
|
assert setting.isDeprecated() && setting.exists(this) : setting.getKey();
|
||||||
|
return deprecatedSettings.add(setting.getKey());
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Setting names found in this Settings for both string and secure settings.
|
* Setting names found in this Settings for both string and secure settings.
|
||||||
* This is constructed lazily in {@link #keySet()}.
|
* This is constructed lazily in {@link #keySet()}.
|
||||||
|
|
|
@ -154,6 +154,22 @@ public class SettingTests extends ESTestCase {
|
||||||
assertNull(ab2.get());
|
assertNull(ab2.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void testDeprecatedSetting() {
|
||||||
|
final Setting<Boolean> deprecatedSetting = Setting.boolSetting("deprecated.foo.bar", false, Property.Deprecated);
|
||||||
|
final Settings settings = Settings.builder().put("deprecated.foo.bar", true).build();
|
||||||
|
final int iterations = randomIntBetween(0, 128);
|
||||||
|
for (int i = 0; i < iterations; i++) {
|
||||||
|
deprecatedSetting.get(settings);
|
||||||
|
}
|
||||||
|
if (iterations > 0) {
|
||||||
|
/*
|
||||||
|
* This tests that we log the deprecation warning exactly one time, otherwise we would have to assert the deprecation warning
|
||||||
|
* for each usage of the setting.
|
||||||
|
*/
|
||||||
|
assertSettingDeprecationsAndWarnings(new Setting[]{deprecatedSetting});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public void testDefault() {
|
public void testDefault() {
|
||||||
TimeValue defaultValue = TimeValue.timeValueMillis(randomIntBetween(0, 1000000));
|
TimeValue defaultValue = TimeValue.timeValueMillis(randomIntBetween(0, 1000000));
|
||||||
Setting<TimeValue> setting =
|
Setting<TimeValue> setting =
|
||||||
|
|
|
@ -30,6 +30,7 @@ import org.elasticsearch.common.settings.Settings;
|
||||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||||
import org.elasticsearch.common.xcontent.json.JsonXContent;
|
import org.elasticsearch.common.xcontent.json.JsonXContent;
|
||||||
import org.elasticsearch.plugins.Plugin;
|
import org.elasticsearch.plugins.Plugin;
|
||||||
|
import org.elasticsearch.test.ESIntegTestCase;
|
||||||
import org.hamcrest.Matcher;
|
import org.hamcrest.Matcher;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
@ -54,6 +55,7 @@ import static org.hamcrest.Matchers.hasSize;
|
||||||
/**
|
/**
|
||||||
* Tests {@code DeprecationLogger} uses the {@code ThreadContext} to add response headers.
|
* Tests {@code DeprecationLogger} uses the {@code ThreadContext} to add response headers.
|
||||||
*/
|
*/
|
||||||
|
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST)
|
||||||
public class DeprecationHttpIT extends HttpSmokeTestCase {
|
public class DeprecationHttpIT extends HttpSmokeTestCase {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
@ -125,14 +127,6 @@ public class DeprecationHttpIT extends HttpSmokeTestCase {
|
||||||
doTestDeprecationWarningsAppearInHeaders();
|
doTestDeprecationWarningsAppearInHeaders();
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testDeprecationHeadersDoNotGetStuck() throws Exception {
|
|
||||||
doTestDeprecationWarningsAppearInHeaders();
|
|
||||||
doTestDeprecationWarningsAppearInHeaders();
|
|
||||||
if (rarely()) {
|
|
||||||
doTestDeprecationWarningsAppearInHeaders();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Run a request that receives a predictably randomized number of deprecation warnings.
|
* Run a request that receives a predictably randomized number of deprecation warnings.
|
||||||
* <p>
|
* <p>
|
||||||
|
|
Loading…
Reference in New Issue