Skip to content

Commit

Permalink
Fixed failing tests due to deprecated core setting warning
Browse files Browse the repository at this point in the history 
Signed-off-by: Martin Gaievski <[email protected]>
  • Loading branch information
@martin-gaievski
martin-gaievski committed Aug 23, 2023
1 parent 9702dd0 commit 53085da
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 30 deletions.
21 changes: 13 additions & 8 deletions src/main/java/org/opensearch/knn/plugin/script/KNNScoreScript.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.opensearch.knn.plugin.script;

import org.apache.lucene.search.IndexSearcher;
import org.opensearch.knn.index.KNNVectorScriptDocValues;
import org.apache.lucene.index.LeafReaderContext;
import org.opensearch.index.fielddata.ScriptDocValues;
Expand Down Expand Up @@ -32,9 +33,10 @@ public KNNScoreScript(
String field,
BiFunction<T, T, Float> scoringMethod,
SearchLookup lookup,
LeafReaderContext leafContext
LeafReaderContext leafContext,
IndexSearcher searcher
) {
super(params, lookup, leafContext);
super(params, lookup, searcher, leafContext);
this.queryValue = queryValue;
this.field = field;
this.scoringMethod = scoringMethod;
Expand All @@ -51,9 +53,10 @@ public LongType(
String field,
BiFunction<Long, Long, Float> scoringMethod,
SearchLookup lookup,
LeafReaderContext leafContext
LeafReaderContext leafContext,
IndexSearcher searcher
) {
super(params, queryValue, field, scoringMethod, lookup, leafContext);
super(params, queryValue, field, scoringMethod, lookup, leafContext, searcher);
}

/**
Expand Down Expand Up @@ -84,9 +87,10 @@ public BigIntegerType(
String field,
BiFunction<BigInteger, BigInteger, Float> scoringMethod,
SearchLookup lookup,
LeafReaderContext leafContext
LeafReaderContext leafContext,
IndexSearcher searcher
) {
super(params, queryValue, field, scoringMethod, lookup, leafContext);
super(params, queryValue, field, scoringMethod, lookup, leafContext, searcher);
}

/**
Expand Down Expand Up @@ -118,9 +122,10 @@ public KNNVectorType(
String field,
BiFunction<float[], float[], Float> scoringMethod,
SearchLookup lookup,
LeafReaderContext leafContext
LeafReaderContext leafContext,
IndexSearcher searcher
) throws IOException {
super(params, queryValue, field, scoringMethod, lookup, leafContext);
super(params, queryValue, field, scoringMethod, lookup, leafContext, searcher);
}

/**
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/org/opensearch/knn/plugin/script/KNNScoreScriptFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.opensearch.knn.plugin.script;

import org.apache.lucene.search.IndexSearcher;
import org.opensearch.knn.plugin.stats.KNNCounter;
import org.apache.lucene.index.LeafReaderContext;
import org.opensearch.script.ScoreScript;
Expand All @@ -20,14 +21,16 @@ public class KNNScoreScriptFactory implements ScoreScript.LeafFactory {
private String field;
private Object query;
private KNNScoringSpace knnScoringSpace;
private IndexSearcher searcher;

public KNNScoreScriptFactory(Map<String, Object> params, SearchLookup lookup) {
public KNNScoreScriptFactory(Map<String, Object> params, SearchLookup lookup, IndexSearcher searcher) {
KNNCounter.SCRIPT_QUERY_REQUESTS.increment();
this.params = params;
this.lookup = lookup;
this.field = getValue(params, "field").toString();
this.similaritySpace = getValue(params, "space_type").toString();
this.query = getValue(params, "query_value");
this.searcher = searcher;

this.knnScoringSpace = KNNScoringSpaceFactory.create(
this.similaritySpace,
Expand Down Expand Up @@ -60,6 +63,6 @@ public boolean needs_score() {
*/
@Override
public ScoreScript newInstance(LeafReaderContext ctx) throws IOException {
return knnScoringSpace.getScoreScript(params, field, lookup, ctx);
return knnScoringSpace.getScoreScript(params, field, lookup, ctx, this.searcher);
}
}
74 changes: 54 additions & 20 deletions src/main/java/org/opensearch/knn/plugin/script/KNNScoringSpace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.opensearch.knn.plugin.script;

import org.apache.lucene.search.IndexSearcher;
import org.opensearch.knn.index.mapper.KNNVectorFieldMapper;
import org.opensearch.knn.index.query.KNNWeight;
import org.apache.lucene.index.LeafReaderContext;
Expand Down Expand Up @@ -36,7 +37,8 @@ public interface KNNScoringSpace {
* @return ScoreScript for this query
* @throws IOException throws IOException if ScoreScript cannot be constructed
*/
ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx) throws IOException;
ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx, IndexSearcher searcher)
throws IOException;

class L2 implements KNNScoringSpace {

Expand All @@ -62,9 +64,14 @@ public L2(Object query, MappedFieldType fieldType) {
this.scoringMethod = (float[] q, float[] v) -> 1 / (1 + KNNScoringUtil.l2Squared(q, v));
}

public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx);
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}
}

Expand Down Expand Up @@ -94,9 +101,14 @@ public CosineSimilarity(Object query, MappedFieldType fieldType) {
this.scoringMethod = (float[] q, float[] v) -> 1 + KNNScoringUtil.cosinesimilOptimized(q, v, qVectorSquaredMagnitude);
}

public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx);
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}
}

Expand Down Expand Up @@ -127,16 +139,22 @@ public HammingBit(Object query, MappedFieldType fieldType) {
}

@SuppressWarnings("unchecked")
public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
if (this.processedQuery instanceof Long) {
return new KNNScoreScript.LongType(
params,
(Long) this.processedQuery,
field,
(BiFunction<Long, Long, Float>) this.scoringMethod,
lookup,
ctx
ctx,
searcher
);
}

Expand All @@ -146,7 +164,8 @@ public ScoreScript getScoreScript(Map<String, Object> params, String field, Sear
field,
(BiFunction<BigInteger, BigInteger, Float>) this.scoringMethod,
lookup,
ctx
ctx,
searcher
);
}
}
Expand Down Expand Up @@ -175,9 +194,14 @@ public L1(Object query, MappedFieldType fieldType) {
this.scoringMethod = (float[] q, float[] v) -> 1 / (1 + KNNScoringUtil.l1Norm(q, v));
}

public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx);
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}
}

Expand Down Expand Up @@ -205,9 +229,14 @@ public LInf(Object query, MappedFieldType fieldType) {
this.scoringMethod = (float[] q, float[] v) -> 1 / (1 + KNNScoringUtil.lInfNorm(q, v));
}

public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx);
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}
}

Expand Down Expand Up @@ -238,9 +267,14 @@ public InnerProd(Object query, MappedFieldType fieldType) {
}

@Override
public ScoreScript getScoreScript(Map<String, Object> params, String field, SearchLookup lookup, LeafReaderContext ctx)
throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx);
public ScoreScript getScoreScript(
Map<String, Object> params,
String field,
SearchLookup lookup,
LeafReaderContext ctx,
IndexSearcher searcher
) throws IOException {
return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}
}
}
15 changes: 15 additions & 0 deletions src/test/java/org/opensearch/knn/index/KNNSettingsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public void testGetSettingValueFromConfig() {
.getSettingValue(KNNSettings.KNN_MEMORY_CIRCUIT_BREAKER_LIMIT)).getKb();
mockNode.close();
assertEquals(expectedKNNCircuitBreakerLimit, actualKNNCircuitBreakerLimit);
assertWarnings();
}

@SneakyThrows
Expand All @@ -63,6 +64,10 @@ public void testGetSettingValueDefault() {
actualKNNCircuitBreakerLimit

);
// set warning for deprecation of index.store.hybrid.mmap.extensions as expected temporarily, need to work on proper strategy of
// switching to new setting in core
// no-jdk distributions expected warning is a workaround for running tests locally
assertWarnings();
}

private Node createMockNode(Map<String, Object> configSettings) throws IOException {
Expand Down Expand Up @@ -93,4 +98,14 @@ private static Settings.Builder baseSettings() {
.put(NetworkModule.TRANSPORT_TYPE_KEY, getTestTransportType())
.put(dataNode());
}

private void assertWarnings() {
// set warning for deprecation of index.store.hybrid.mmap.extensions as expected temporarily, need to work on proper strategy of
// switching to new setting in core
// no-jdk distributions expected warning is a workaround for running tests locally
assertWarnings(
"[index.store.hybrid.mmap.extensions] setting was deprecated in OpenSearch and will be removed in a future release! See the breaking changes documentation for the next major version.",
"no-jdk distributions that do not bundle a JDK are deprecated and will be removed in a future release"
);
}
}

0 comments on commit 53085da

Please sign in to comment.