Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix flaky test MoreExpressionIT.testSpecialValueVariable in concurren…
Browse files Browse the repository at this point in the history
…t search path

Signed-off-by: Sorabh Hamirwasia <[email protected]>
sohami committed Nov 4, 2023
1 parent 54fa050 commit 3c9e07b
Showing 2 changed files with 19 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -504,10 +504,6 @@ public void testInvalidFieldMember() {
}

public void testSpecialValueVariable() throws Exception {
assumeFalse(
"Concurrent search case muted pending fix: https://github.com/opensearch-project/OpenSearch/issues/10079",
internalCluster().clusterService().getClusterSettings().get(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING)
);
// i.e. _value for aggregations
createIndex("test");
ensureGreen("test");
Original file line number Diff line number Diff line change
@@ -39,20 +39,25 @@
import org.apache.lucene.search.IndexSearcher;

import java.io.IOException;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

/**
* A {@link DoubleValuesSource} which has a stub {@link DoubleValues} that holds a dynamically replaceable constant double.
* A {@link DoubleValuesSource} which has a stub {@link DoubleValues} that holds a dynamically replaceable constant double. This is made
* thread-safe for concurrent segment search use case by keeping the {@link DoubleValues} per thread. Any update to the value happens in
* thread specific {@link DoubleValuesSource} instance.
*/
final class ReplaceableConstDoubleValueSource extends DoubleValuesSource {
final ReplaceableConstDoubleValues fv;
// Multiple slices can be processed by same thread but that will be sequential, so keeping per thread is fine
final Map<Long, ReplaceableConstDoubleValues> perThreadDoubleValues;

ReplaceableConstDoubleValueSource() {
fv = new ReplaceableConstDoubleValues();
perThreadDoubleValues = new ConcurrentHashMap<>();

Check warning on line 55 in modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java

Codecov / codecov/patch

modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java#L55

Added line #L55 was not covered by tests
}

@Override
public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
return fv;
return perThreadDoubleValues.computeIfAbsent(Thread.currentThread().getId(), threadId -> new ReplaceableConstDoubleValues());

Check warning on line 60 in modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java

Codecov / codecov/patch

modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java#L60

Added line #L60 was not covered by tests
}

@Override
@@ -62,7 +67,11 @@ public boolean needsScores() {

@Override
public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException {
if (fv.advanceExact(docId)) return Explanation.match((float) fv.doubleValue(), "ReplaceableConstDoubleValues");
final ReplaceableConstDoubleValues currentFv = perThreadDoubleValues.computeIfAbsent(
Thread.currentThread().getId(),
threadId -> new ReplaceableConstDoubleValues()

Check warning on line 72 in modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java

Codecov / codecov/patch

modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java#L70-L72

Added lines #L70 - L72 were not covered by tests
);
if (currentFv.advanceExact(docId)) return Explanation.match((float) currentFv.doubleValue(), "ReplaceableConstDoubleValues");
else return Explanation.noMatch("ReplaceableConstDoubleValues");
}

@@ -77,7 +86,11 @@ public int hashCode() {
}

public void setValue(double v) {
fv.setValue(v);
final ReplaceableConstDoubleValues currentFv = perThreadDoubleValues.computeIfAbsent(
Thread.currentThread().getId(),
threadId -> new ReplaceableConstDoubleValues()

Check warning on line 91 in modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java

Codecov / codecov/patch

modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java#L89-L91

Added lines #L89 - L91 were not covered by tests
);
currentFv.setValue(v);

Check warning on line 93 in modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java

Codecov / codecov/patch

modules/lang-expression/src/main/java/org/opensearch/script/expression/ReplaceableConstDoubleValueSource.java#L93

Added line #L93 was not covered by tests
}

@Override

0 comments on commit 3c9e07b

Please sign in to comment.