Skip to content

Commit df687f7

Browse files
committed
Add min version checker
Signed-off-by: John Mazanec <jmazane@amazon.com>
1 parent 019429a commit df687f7

File tree

7 files changed

+30
-19
lines changed

7 files changed

+30
-19
lines changed

qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/StatsIT.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.apache.hc.core5.http.io.entity.EntityUtils;
99
import org.junit.Before;
10+
import org.opensearch.Version;
1011
import org.opensearch.client.Response;
1112
import org.opensearch.client.ResponseException;
1213
import org.opensearch.knn.plugin.stats.KNNStats;
@@ -21,7 +22,7 @@ public class StatsIT extends AbstractRollingUpgradeTestCase {
2122
@Before
2223
public void setUp() throws Exception {
2324
super.setUp();
24-
this.knnStats = new KNNStats(null);
25+
this.knnStats = new KNNStats(null, () -> Version.CURRENT);
2526
}
2627

2728
// Validate if all the KNN Stats metrics from old version are present in new version

src/main/java/org/opensearch/knn/plugin/KNNPlugin.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public Collection<Object> createComponents(
215215

216216
clusterService.addListener(TrainingJobClusterStateListener.getInstance());
217217

218-
KNNStats knnStats = new KNNStats(client);
218+
KNNStats knnStats = new KNNStats(client, () -> clusterService.getClusterManagerService().getMinNodeVersion());
219219
return ImmutableList.of(knnStats);
220220
}
221221

src/main/java/org/opensearch/knn/plugin/stats/CircuitBreakerStat.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55

66
package org.opensearch.knn.plugin.stats;
77

8+
import org.opensearch.Version;
89
import org.opensearch.core.action.ActionListener;
10+
import org.opensearch.knn.index.KNNSettings;
911
import org.opensearch.knn.plugin.transport.KNNCircuitBreakerTrippedAction;
1012
import org.opensearch.knn.plugin.transport.KNNCircuitBreakerTrippedRequest;
1113
import org.opensearch.transport.client.Client;
1214

1315
import java.util.Map;
1416
import java.util.function.Function;
17+
import java.util.function.Supplier;
18+
19+
import static org.opensearch.knn.index.KNNSettings.KNN_CIRCUIT_BREAKER_TRIGGERED;
1520

1621
public class CircuitBreakerStat extends KNNStat<Boolean> {
1722

@@ -25,14 +30,26 @@ public class CircuitBreakerStat extends KNNStat<Boolean> {
2530
};
2631

2732
private final Client client;
33+
private final Supplier<Version> minVersionSupplier;
2834

29-
public CircuitBreakerStat(Client client) {
35+
public CircuitBreakerStat(Client client, Supplier<Version> minVersionSupplier) {
3036
super(true, FETCHER);
3137
this.client = client;
38+
this.minVersionSupplier = minVersionSupplier;
3239
}
3340

3441
@Override
3542
public ActionListener<Void> setupContext(KNNStatFetchContext knnStatFetchContext, ActionListener<Void> actionListener) {
43+
// If there are any nodes in the cluster before 3.0, then we need to fall back to checking the CB
44+
if (minVersionSupplier.get().compareTo(Version.V_3_0_0) < 0) {
45+
return ActionListener.wrap(knnCircuitBreakerTrippedResponse -> {
46+
knnStatFetchContext.addContext(
47+
StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(),
48+
Map.of(CONTEXT_CB_TRIPPED, KNNSettings.state().getSettingValue(KNN_CIRCUIT_BREAKER_TRIGGERED))
49+
);
50+
actionListener.onResponse(null);
51+
}, actionListener::onFailure);
52+
}
3653
return ActionListener.wrap(
3754
response -> client.execute(
3855
KNNCircuitBreakerTrippedAction.INSTANCE,

src/main/java/org/opensearch/knn/plugin/stats/KNNStats.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import com.google.common.cache.CacheStats;
99
import com.google.common.collect.ImmutableMap;
10+
import org.opensearch.Version;
1011
import org.opensearch.knn.common.KNNConstants;
1112
import org.opensearch.knn.index.memory.NativeMemoryCacheManager;
1213
import org.opensearch.knn.index.engine.KNNEngine;
@@ -33,13 +34,15 @@
3334
public class KNNStats {
3435

3536
private final Client client;
37+
private final Supplier<Version> minVersionSupplier;
3638
private final Map<String, KNNStat<?>> knnStats;
3739

3840
/**
3941
* Constructor
4042
*/
41-
public KNNStats(Client client) {
43+
public KNNStats(Client client, Supplier<Version> minVersionSupplier) {
4244
this.client = client;
45+
this.minVersionSupplier = minVersionSupplier;
4346
this.knnStats = buildStatsMap();
4447
}
4548

@@ -148,7 +151,7 @@ private void addNativeMemoryStats(ImmutableMap.Builder<String, KNNStat<?>> build
148151
.put(StatNames.GRAPH_QUERY_REQUESTS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_QUERY_REQUESTS)))
149152
.put(StatNames.GRAPH_INDEX_ERRORS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_INDEX_ERRORS)))
150153
.put(StatNames.GRAPH_INDEX_REQUESTS.getName(), new KNNStat<>(false, new KNNCounterSupplier(KNNCounter.GRAPH_INDEX_REQUESTS)))
151-
.put(StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(), new CircuitBreakerStat(client));
154+
.put(StatNames.CIRCUIT_BREAKER_TRIGGERED.getName(), new CircuitBreakerStat(client, minVersionSupplier));
152155
}
153156

154157
private void addEngineStats(ImmutableMap.Builder<String, KNNStat<?>> builder) {

src/test/java/org/opensearch/knn/index/KNNCircuitBreakerIT.java

-12
Original file line numberDiff line numberDiff line change
@@ -151,18 +151,6 @@ public void testCbTripped() throws Exception {
151151
}
152152

153153
public void verifyCbUntrips() throws Exception {
154-
if (!isCbTripped()) {
155-
// Set cluster-level limit to 1kb (half of what indices require)
156-
updateClusterSettings("knn.memory.circuit_breaker.limit", "1kb");
157-
158-
// Load indices into cache
159-
search(INDEX_1, INDEX_2);
160-
161-
// Verify circuit breaker tripped
162-
Thread.sleep(5 * 1000);
163-
assertTrue(isCbTripped());
164-
}
165-
166154
int backOffInterval = 5; // seconds
167155
for (int i = 0; i < CB_TIME_INTERVAL; i += backOffInterval) {
168156
if (!isCbTripped()) {

src/test/java/org/opensearch/knn/plugin/action/RestKNNStatsHandlerIT.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.apache.hc.core5.http.io.entity.EntityUtils;
1010
import org.junit.Before;
1111
import org.junit.rules.DisableOnDebug;
12+
import org.opensearch.Version;
1213
import org.opensearch.client.Request;
1314
import org.opensearch.client.Response;
1415
import org.opensearch.client.ResponseException;
@@ -75,7 +76,7 @@ public class RestKNNStatsHandlerIT extends KNNRestTestCase {
7576

7677
@Before
7778
public void setup() {
78-
knnStats = new KNNStats(null);
79+
knnStats = new KNNStats(null, () -> Version.CURRENT);
7980
}
8081

8182
/**

src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package org.opensearch.knn.plugin.action;
1313

14+
import org.opensearch.Version;
1415
import org.opensearch.knn.KNNRestTestCase;
1516
import org.opensearch.knn.index.KNNSettings;
1617
import org.opensearch.knn.index.SpaceType;
@@ -49,7 +50,7 @@ public class RestLegacyKNNStatsHandlerIT extends KNNRestTestCase {
4950

5051
@Before
5152
public void setup() {
52-
knnStats = new KNNStats(null);
53+
knnStats = new KNNStats(null, () -> Version.CURRENT);
5354
}
5455

5556
/**

0 commit comments

Comments
 (0)