Skip to content

Commit

Permalink
Use holder pattern for lazy deprecation loggers
Browse files Browse the repository at this point in the history
In a few places we need to lazy initialize static deprecation
loggers. This is needed to avoid touching logging before logging is
configured, but deprecation loggers that are used in foundational
classes like settings and parsers would be initialized before logging is
configured. Previously we used a lazy set once pattern which is fine,
but there's a simpler approach: the holder pattern.

Relates #26218
  • Loading branch information
jasontedor committed Aug 15, 2017
1 parent 17ad42a commit 37e67c9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 15 deletions.
Expand Up @@ -19,16 +19,13 @@
package org.elasticsearch.common.settings;

import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.action.support.ToXContentToBytes;
import org.elasticsearch.common.Booleans;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.MemorySizeValue;
Expand Down Expand Up @@ -394,23 +391,13 @@ public String getRaw(Settings settings) {
return settings.get(getKey(), defaultValue.apply(settings));
}

private static SetOnce<DeprecationLogger> deprecationLogger = new SetOnce<>();

// we have to initialize lazily otherwise a logger would be constructed before logging is initialized
private static synchronized DeprecationLogger getDeprecationLogger() {
if (deprecationLogger.get() == null) {
deprecationLogger.set(new DeprecationLogger(Loggers.getLogger(Settings.class)));
}
return deprecationLogger.get();
}

/** Logs a deprecation warning if the setting is deprecated and used. */
void checkDeprecation(Settings settings) {
// They're using the setting, so we need to tell them to stop
if (this.isDeprecated() && this.exists(settings)) {
// It would be convenient to show its replacement key, but replacement is often not so simple
final String key = getKey();
getDeprecationLogger().deprecatedAndMaybeLog(
Settings.DeprecationLoggerHolder.deprecationLogger.deprecatedAndMaybeLog(
key,
"[{}] setting was deprecated in Elasticsearch and will be removed in a future release! "
+ "See the breaking changes documentation for the next major version.",
Expand Down
Expand Up @@ -27,6 +27,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.loader.SettingsLoader;
import org.elasticsearch.common.settings.loader.SettingsLoaderFactory;
import org.elasticsearch.common.unit.ByteSizeUnit;
Expand Down Expand Up @@ -62,7 +64,6 @@
import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Predicate;
Expand Down Expand Up @@ -320,6 +321,15 @@ public Long getAsLong(String setting, Long defaultValue) {
}
}

/**
* We have to lazy initialize the deprecation logger as otherwise a static logger here would be constructed before logging is configured
* leading to a runtime failure (see {@link LogConfigurator#checkErrorListener()} ). The premature construction would come from any
* {@link Setting} object constructed in, for example, {@link org.elasticsearch.env.Environment}.
*/
static class DeprecationLoggerHolder {
static DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(Settings.class));
}

/**
* Returns the setting value (as boolean) associated with the setting key. If it does not exists,
* returns the default value provided.
Expand Down

0 comments on commit 37e67c9

Please sign in to comment.