Skip to content

Commit

Permalink
Fix log flooding for engine deprecation notice (opensearch-project#2495
Browse files Browse the repository at this point in the history
…) (opensearch-project#2496)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit c8b6637)

Co-authored-by: Kunal Kotwani <kkotwani@amazon.com>
  • Loading branch information
1 parent ab33538 commit 0df6e41
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 31 deletions.
24 changes: 18 additions & 6 deletions src/main/java/org/opensearch/knn/index/engine/EngineResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand All @@ -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;
}
}
10 changes: 1 addition & 9 deletions src/main/java/org/opensearch/knn/index/engine/KNNEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -37,7 +35,7 @@ public enum KNNEngine implements KNNLibrary {
private static final Set<KNNEngine> CUSTOM_SEGMENT_FILE_ENGINES = ImmutableSet.of(KNNEngine.NMSLIB, KNNEngine.FAISS);
private static final Set<KNNEngine> ENGINES_SUPPORTING_FILTERS = ImmutableSet.of(KNNEngine.LUCENE, KNNEngine.FAISS);
public static final Set<KNNEngine> ENGINES_SUPPORTING_RADIAL_SEARCH = ImmutableSet.of(KNNEngine.LUCENE, KNNEngine.FAISS);
private static Logger logger = LogManager.getLogger(KNNEngine.class);
public static final Set<KNNEngine> DEPRECATED_ENGINES = ImmutableSet.of(KNNEngine.NMSLIB);

private static Map<KNNEngine, Integer> MAX_DIMENSIONS_BY_ENGINE = Map.of(
KNNEngine.NMSLIB,
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +31,6 @@
public class NmslibMethodResolver extends AbstractMethodResolver {

private static final Set<CompressionLevel> SUPPORTED_COMPRESSION_LEVELS = Set.of(CompressionLevel.x1);
private static Logger logger = LogManager.getLogger(NmslibMethodResolver.class);

@Override
public ResolvedMethodContext resolveMethod(
Expand All @@ -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,
Expand Down
10 changes: 0 additions & 10 deletions src/main/java/org/opensearch/knn/jni/JNIService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -209,9 +202,6 @@ public static long loadIndex(IndexInputWithBuffer readStream, Map<String, Object
return FaissService.loadIndexWithStream(readStream);
}
} else 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."
);
return NmslibService.loadIndexWithStream(readStream, parameters);
}

Expand Down

0 comments on commit 0df6e41

Please sign in to comment.