From 0df6e41aab48f349edcf2a0758d2a256288f8417 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 6 Feb 2025 11:54:16 -0800 Subject: [PATCH] Fix log flooding for engine deprecation notice (#2495) (#2496) Signed-off-by: Kunal Kotwani (cherry picked from commit c8b6637b7322bfa03e71908104023bf32d28b9c7) Co-authored-by: Kunal Kotwani --- .../knn/index/engine/EngineResolver.java | 24 ++++++++++++++----- .../knn/index/engine/KNNEngine.java | 10 +------- .../engine/nmslib/NmslibMethodResolver.java | 6 ----- .../org/opensearch/knn/jni/JNIService.java | 10 -------- 4 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/opensearch/knn/index/engine/EngineResolver.java b/src/main/java/org/opensearch/knn/index/engine/EngineResolver.java index d52c21c4c..983adf28b 100644 --- a/src/main/java/org/opensearch/knn/index/engine/EngineResolver.java +++ b/src/main/java/org/opensearch/knn/index/engine/EngineResolver.java @@ -5,14 +5,19 @@ package org.opensearch.knn.index.engine; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.knn.index.mapper.CompressionLevel; import org.opensearch.knn.index.mapper.Mode; +import static org.opensearch.knn.index.engine.KNNEngine.DEPRECATED_ENGINES; + /** * Figures out what {@link KNNEngine} to use based on configuration details */ public final class EngineResolver { + private static Logger logger = LogManager.getLogger(EngineResolver.class); public static final EngineResolver INSTANCE = new EngineResolver(); private EngineResolver() {} @@ -32,31 +37,38 @@ public KNNEngine resolveEngine( ) { // User configuration gets precedence if (knnMethodContext != null && knnMethodContext.isEngineConfigured()) { - return knnMethodContext.getKnnEngine(); + return logAndReturnEngine(knnMethodContext.getKnnEngine()); } // Faiss is the only engine that supports training, so we default to faiss here for now if (requiresTraining) { - return KNNEngine.FAISS; + return logAndReturnEngine(KNNEngine.FAISS); } Mode mode = knnMethodConfigContext.getMode(); CompressionLevel compressionLevel = knnMethodConfigContext.getCompressionLevel(); // If both mode and compression are not specified, we can just default if (Mode.isConfigured(mode) == false && CompressionLevel.isConfigured(compressionLevel) == false) { - return KNNEngine.DEFAULT; + return logAndReturnEngine(KNNEngine.DEFAULT); } // For 1x, we need to default to faiss if mode is provided and use nmslib otherwise if (CompressionLevel.isConfigured(compressionLevel) == false || compressionLevel == CompressionLevel.x1) { - return mode == Mode.ON_DISK ? KNNEngine.FAISS : KNNEngine.NMSLIB; + return logAndReturnEngine(mode == Mode.ON_DISK ? KNNEngine.FAISS : KNNEngine.NMSLIB); } // Lucene is only engine that supports 4x - so we have to default to it here. if (compressionLevel == CompressionLevel.x4) { - return KNNEngine.LUCENE; + return logAndReturnEngine(KNNEngine.LUCENE); } - return KNNEngine.FAISS; + return logAndReturnEngine(KNNEngine.FAISS); + } + + private KNNEngine logAndReturnEngine(KNNEngine knnEngine) { + if (DEPRECATED_ENGINES.contains(knnEngine)) { + logger.warn("[Deprecation] {} engine is deprecated and will be removed in a future release.", knnEngine); + } + return knnEngine; } } diff --git a/src/main/java/org/opensearch/knn/index/engine/KNNEngine.java b/src/main/java/org/opensearch/knn/index/engine/KNNEngine.java index ea06d985f..05858cefc 100644 --- a/src/main/java/org/opensearch/knn/index/engine/KNNEngine.java +++ b/src/main/java/org/opensearch/knn/index/engine/KNNEngine.java @@ -6,8 +6,6 @@ package org.opensearch.knn.index.engine; import com.google.common.collect.ImmutableSet; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.common.ValidationException; import org.opensearch.knn.index.SpaceType; import org.opensearch.knn.index.engine.faiss.Faiss; @@ -37,7 +35,7 @@ public enum KNNEngine implements KNNLibrary { private static final Set CUSTOM_SEGMENT_FILE_ENGINES = ImmutableSet.of(KNNEngine.NMSLIB, KNNEngine.FAISS); private static final Set ENGINES_SUPPORTING_FILTERS = ImmutableSet.of(KNNEngine.LUCENE, KNNEngine.FAISS); public static final Set ENGINES_SUPPORTING_RADIAL_SEARCH = ImmutableSet.of(KNNEngine.LUCENE, KNNEngine.FAISS); - private static Logger logger = LogManager.getLogger(KNNEngine.class); + public static final Set DEPRECATED_ENGINES = ImmutableSet.of(KNNEngine.NMSLIB); private static Map MAX_DIMENSIONS_BY_ENGINE = Map.of( KNNEngine.NMSLIB, @@ -70,9 +68,6 @@ public enum KNNEngine implements KNNLibrary { */ public static KNNEngine getEngine(String name) { if (NMSLIB.getName().equalsIgnoreCase(name)) { - logger.warn( - "[Deprecation] nmslib engine is deprecated and will be removed in a future release. Please use Faiss or Lucene engine instead." - ); return NMSLIB; } @@ -95,9 +90,6 @@ public static KNNEngine getEngine(String name) { */ public static KNNEngine getEngineNameFromPath(String path) { if (path.endsWith(KNNEngine.NMSLIB.getExtension()) || path.endsWith(KNNEngine.NMSLIB.getCompoundExtension())) { - logger.warn( - "[Deprecation] nmslib engine is deprecated and will be removed in a future release. Please use Faiss or Lucene engine instead." - ); return KNNEngine.NMSLIB; } diff --git a/src/main/java/org/opensearch/knn/index/engine/nmslib/NmslibMethodResolver.java b/src/main/java/org/opensearch/knn/index/engine/nmslib/NmslibMethodResolver.java index ef29b7686..70d14ab8d 100644 --- a/src/main/java/org/opensearch/knn/index/engine/nmslib/NmslibMethodResolver.java +++ b/src/main/java/org/opensearch/knn/index/engine/nmslib/NmslibMethodResolver.java @@ -5,8 +5,6 @@ package org.opensearch.knn.index.engine.nmslib; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.common.ValidationException; import org.opensearch.knn.index.SpaceType; import org.opensearch.knn.index.engine.AbstractMethodResolver; @@ -33,7 +31,6 @@ public class NmslibMethodResolver extends AbstractMethodResolver { private static final Set SUPPORTED_COMPRESSION_LEVELS = Set.of(CompressionLevel.x1); - private static Logger logger = LogManager.getLogger(NmslibMethodResolver.class); @Override public ResolvedMethodContext resolveMethod( @@ -42,9 +39,6 @@ public ResolvedMethodContext resolveMethod( boolean shouldRequireTraining, final SpaceType spaceType ) { - logger.warn( - "[Deprecation] nmslib engine is deprecated and will be removed in a future release. Please use Faiss or Lucene engine instead." - ); validateConfig(knnMethodConfigContext, shouldRequireTraining); KNNMethodContext resolvedKNNMethodContext = initResolvedKNNMethodContext( knnMethodContext, diff --git a/src/main/java/org/opensearch/knn/jni/JNIService.java b/src/main/java/org/opensearch/knn/jni/JNIService.java index 3d2a13f97..ec8b377c3 100644 --- a/src/main/java/org/opensearch/knn/jni/JNIService.java +++ b/src/main/java/org/opensearch/knn/jni/JNIService.java @@ -12,8 +12,6 @@ package org.opensearch.knn.jni; import org.apache.commons.lang.ArrayUtils; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.common.Nullable; import org.opensearch.knn.common.KNNConstants; import org.opensearch.knn.index.engine.KNNEngine; @@ -30,8 +28,6 @@ */ public class JNIService { - private static Logger logger = LogManager.getLogger(JNIService.class); - /** * Initialize an index for the native library. Takes in numDocs to * allocate the correct amount of memory. @@ -142,9 +138,6 @@ public static void createIndex( KNNEngine knnEngine ) { if (KNNEngine.NMSLIB == knnEngine) { - logger.warn( - "[Deprecation] nmslib engine is deprecated and will be removed in a future release. Please use Faiss or Lucene engine instead." - ); NmslibService.createIndex(ids, vectorsAddress, dim, output, parameters); return; } @@ -209,9 +202,6 @@ public static long loadIndex(IndexInputWithBuffer readStream, Map