Browse Source

Use holder pattern for lazy deprecation loggers

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
Jason Tedor 8 years ago
parent
commit
d1780a8052

+ 1 - 14
core/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -19,7 +19,6 @@
 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;
@@ -27,8 +26,6 @@ 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;
@@ -394,23 +391,13 @@ public class Setting<T> extends ToXContentToBytes {
         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.",

+ 11 - 1
core/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -27,6 +27,8 @@ import org.elasticsearch.common.io.Streams;
 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;
@@ -62,7 +64,6 @@ import java.util.NoSuchElementException;
 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;
@@ -320,6 +321,15 @@ public final class Settings implements ToXContent {
         }
     }
 
+    /**
+     * 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.