Add min version checker

Signed-off-by: John Mazanec <[email protected]>
opensearch-project · Feb 27, 2025 · df687f7 · df687f7
1 parent 019429a
commit df687f7
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 19 deletions.
diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/StatsIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/StatsIT.java
@@ -7,6 +7,7 @@
 
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.junit.Before;
+import org.opensearch.Version;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
 import org.opensearch.knn.plugin.stats.KNNStats;
@@ -21,7 +22,7 @@ public class StatsIT extends AbstractRollingUpgradeTestCase {
     @Before
     public void setUp() throws Exception {
         super.setUp();
-        this.knnStats = new KNNStats(null);
+        this.knnStats = new KNNStats(null, () -> Version.CURRENT);
     }
 
     // Validate if all the KNN Stats metrics from old version are present in new version

diff --git a/src/main/java/org/opensearch/knn/plugin/KNNPlugin.java b/src/main/java/org/opensearch/knn/plugin/KNNPlugin.java
@@ -215,7 +215,7 @@ public Collection<Object> createComponents(
 
         clusterService.addListener(TrainingJobClusterStateListener.getInstance());
 
-        KNNStats knnStats = new KNNStats(client);
+        KNNStats knnStats = new KNNStats(client, () -> clusterService.getClusterManagerService().getMinNodeVersion());
         return ImmutableList.of(knnStats);
     }
 

diff --git a/src/main/java/org/opensearch/knn/plugin/stats/CircuitBreakerStat.java b/src/main/java/org/opensearch/knn/plugin/stats/CircuitBreakerStat.java
@@ -5,13 +5,18 @@
 
 package org.opensearch.knn.plugin.stats;
 
+import org.opensearch.Version;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.plugin.transport.KNNCircuitBreakerTrippedAction;
 import org.opensearch.knn.plugin.transport.KNNCircuitBreakerTrippedRequest;
 import org.opensearch.transport.client.Client;
 
 import java.util.Map;
 import java.util.function.Function;
+import java.util.function.Supplier;
+
+import static org.opensearch.knn.index.KNNSettings.KNN_CIRCUIT_BREAKER_TRIGGERED;
 
 public class CircuitBreakerStat extends KNNStat<Boolean> {
 
@@ -25,14 +30,26 @@ public class CircuitBreakerStat extends KNNStat<Boolean> {
     };
 
     private final Client client;
+    private final Supplier<Version> minVersionSupplier;
 
-    public CircuitBreakerStat(Client client) {
+    public CircuitBreakerStat(Client client, Supplier<Version> minVersionSupplier) {
         super(true, FETCHER);
         this.client = client;
+        this.minVersionSupplier = minVersionSupplier;
     }
 
     @Override
     public ActionListener<Void> setupContext(KNNStatFetchContext knnStatFetchContext, ActionListener<Void> actionListener) {
+        // If there are any nodes in the cluster before 3.0, then we need to fall back to checking the CB
+        if (minVersionSupplier.get().compareTo(Version.V_3_0_0) < 0) {
+            return ActionListener.wrap(knnCircuitBreakerTrippedResponse -> {
+                knnStatFetchContext.addContext(
+                    StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(),
+                    Map.of(CONTEXT_CB_TRIPPED, KNNSettings.state().getSettingValue(KNN_CIRCUIT_BREAKER_TRIGGERED))
+                );
+                actionListener.onResponse(null);
+            }, actionListener::onFailure);
+        }
         return ActionListener.wrap(
             response -> client.execute(
                 KNNCircuitBreakerTrippedAction.INSTANCE,

diff --git a/src/main/java/org/opensearch/knn/plugin/stats/KNNStats.java b/src/main/java/org/opensearch/knn/plugin/stats/KNNStats.java
@@ -7,6 +7,7 @@
 
 import com.google.common.cache.CacheStats;
 import com.google.common.collect.ImmutableMap;
+import org.opensearch.Version;
 import org.opensearch.knn.common.KNNConstants;
 import org.opensearch.knn.index.memory.NativeMemoryCacheManager;
 import org.opensearch.knn.index.engine.KNNEngine;
@@ -33,13 +34,15 @@
 public class KNNStats {
 
     private final Client client;
+    private final Supplier<Version> minVersionSupplier;
     private final Map<String, KNNStat<?>> knnStats;
 
     /**
      * Constructor
      */
-    public KNNStats(Client client) {
+    public KNNStats(Client client, Supplier<Version> minVersionSupplier) {
         this.client = client;
+        this.minVersionSupplier = minVersionSupplier;
         this.knnStats = buildStatsMap();
     }
 
@@ -148,7 +151,7 @@ private void addNativeMemoryStats(ImmutableMap.Builder<String, KNNStat<?>> build
             .put(StatNames.GRAPH_QUERY_REQUESTS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_QUERY_REQUESTS)))
             .put(StatNames.GRAPH_INDEX_ERRORS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_INDEX_ERRORS)))
             .put(StatNames.GRAPH_INDEX_REQUESTS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_INDEX_REQUESTS)))
-            .put(StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(), new CircuitBreakerStat(client));
+            .put(StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(), new CircuitBreakerStat(client, minVersionSupplier));
     }
 
     private void addEngineStats(ImmutableMap.Builder<String, KNNStat<?>> builder) {

diff --git a/src/test/java/org/opensearch/knn/index/KNNCircuitBreakerIT.java b/src/test/java/org/opensearch/knn/index/KNNCircuitBreakerIT.java
@@ -151,18 +151,6 @@ public void testCbTripped() throws Exception {
     }
 
     public void verifyCbUntrips() throws Exception {
-        if (!isCbTripped()) {
-            // Set cluster-level limit to 1kb (half of what indices require)
-            updateClusterSettings("knn.memory.circuit_breaker.limit", "1kb");
-
-            // Load indices into cache
-            search(INDEX_1, INDEX_2);
-
-            // Verify circuit breaker tripped
-            Thread.sleep(5 * 1000);
-            assertTrue(isCbTripped());
-        }
-
         int backOffInterval = 5; // seconds
         for (int i = 0; i < CB_TIME_INTERVAL; i += backOffInterval) {
             if (!isCbTripped()) {

diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestKNNStatsHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestKNNStatsHandlerIT.java
@@ -9,6 +9,7 @@
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.junit.Before;
 import org.junit.rules.DisableOnDebug;
+import org.opensearch.Version;
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
@@ -75,7 +76,7 @@ public class RestKNNStatsHandlerIT extends KNNRestTestCase {
 
     @Before
     public void setup() {
-        knnStats = new KNNStats(null);
+        knnStats = new KNNStats(null, () -> Version.CURRENT);
     }
 
     /**

diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java
@@ -11,6 +11,7 @@
 
 package org.opensearch.knn.plugin.action;
 
+import org.opensearch.Version;
 import org.opensearch.knn.KNNRestTestCase;
 import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.SpaceType;
@@ -49,7 +50,7 @@ public class RestLegacyKNNStatsHandlerIT extends KNNRestTestCase {
 
     @Before
     public void setup() {
-        knnStats = new KNNStats(null);
+        knnStats = new KNNStats(null, () -> Version.CURRENT);
     }
 
     /**