Skip to content

Commit 89786da

Browse files
ajleong623cwperks
andauthored
Fix Flaky IntervalQueryBuilderTests (#19332)
* fix flaky integration tests Signed-off-by: Anthony Leong <[email protected]> * add unit tests Signed-off-by: Anthony Leong <[email protected]> * retry Signed-off-by: Anthony Leong <[email protected]> * add changelog entry Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> * retry workflows Signed-off-by: Anthony Leong <[email protected]> --------- Signed-off-by: Anthony Leong <[email protected]> Signed-off-by: Craig Perkins <[email protected]> Co-authored-by: Craig Perkins <[email protected]>
1 parent ae140b5 commit 89786da

File tree

5 files changed

+53
-1
lines changed

5 files changed

+53
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
7575
- Remove unnecessary looping in field data cache clear ([#19116](https://github.com/opensearch-project/OpenSearch/pull/19116))
7676
- [Flaky Test] Fix flaky test IngestFromKinesisIT.testAllActiveIngestion ([#19380](https://github.com/opensearch-project/OpenSearch/pull/19380))
7777
- Fix lag metric for pull-based ingestion when streaming source is empty ([#19393](https://github.com/opensearch-project/OpenSearch/pull/19393))
78+
- Fix IntervalQuery flaky test ([#19332](https://github.com/opensearch-project/OpenSearch/pull/19332))
7879
- Fix ingestion state xcontent serialization in IndexMetadata and fail fast on mapping errors([#19320](https://github.com/opensearch-project/OpenSearch/pull/19320))
7980
- Fix updated keyword field params leading to stale responses from request cache ([#19385](https://github.com/opensearch-project/OpenSearch/pull/19385))
8081
- Implement SslHandler retrieval logic for transport-reactor-netty4 plugin ([#19458](https://github.com/opensearch-project/OpenSearch/pull/19458))

server/src/main/java/org/opensearch/index/query/IntervalBuilder.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,20 @@ protected IntervalsSource analyzeTerm(TokenStream ts) throws IOException {
144144
return Intervals.term(BytesRef.deepCopyOf(bytesAtt.getBytesRef()));
145145
}
146146

147+
static boolean canCombineSources(List<IntervalsSource> sources) {
148+
int sourceIndex = 0;
149+
long disjunctionCount = 1;
150+
151+
while (sourceIndex < sources.size()) {
152+
disjunctionCount = disjunctionCount * sources.get(sourceIndex).pullUpDisjunctions().size();
153+
if (disjunctionCount > IndexSearcher.getMaxClauseCount()) {
154+
return false;
155+
}
156+
sourceIndex += 1;
157+
}
158+
return true;
159+
}
160+
147161
protected static IntervalsSource combineSources(List<IntervalsSource> sources, int maxGaps, IntervalMode mode) {
148162
if (sources.size() == 0) {
149163
return NO_INTERVALS;

server/src/main/java/org/opensearch/index/query/IntervalsSourceProvider.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ public IntervalsSource getSource(QueryShardContext ctx, MappedFieldType fieldTyp
443443
for (IntervalsSourceProvider provider : subSources) {
444444
ss.add(provider.getSource(ctx, fieldType));
445445
}
446+
if (maxGaps == 0 && mode == IntervalMode.ORDERED && IntervalBuilder.canCombineSources(ss) == false) {
447+
throw new IllegalArgumentException("Too many disjunctions to expand");
448+
}
446449
IntervalsSource source = IntervalBuilder.combineSources(ss, maxGaps, mode);
447450
if (filter != null) {
448451
return filter.filter(source, ctx, fieldType);

server/src/test/java/org/opensearch/index/query/CombineIntervalsSourceProviderTests.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,17 @@
3232

3333
package org.opensearch.index.query;
3434

35+
import org.apache.lucene.queries.intervals.Intervals;
36+
import org.apache.lucene.queries.intervals.IntervalsSource;
3537
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
3638
import org.opensearch.core.common.io.stream.Writeable;
3739
import org.opensearch.core.xcontent.XContentParser;
40+
import org.opensearch.index.query.IntervalsSourceProvider.Combine;
3841
import org.opensearch.search.SearchModule;
3942
import org.opensearch.test.AbstractSerializingTestCase;
4043

4144
import java.io.IOException;
45+
import java.util.ArrayList;
4246
import java.util.List;
4347

4448
import static org.opensearch.index.query.IntervalsSourceProvider.Combine;
@@ -104,4 +108,28 @@ protected Combine doParseInstance(XContentParser parser) throws IOException {
104108
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
105109
return combine;
106110
}
111+
112+
public void testCanCombineSourcesFail() {
113+
List<IntervalsSource> sources = new ArrayList<>();
114+
115+
for (int i = 0; i < 11; i++) {
116+
IntervalsSource source1 = Intervals.maxgaps(0, Intervals.ordered(Intervals.term("term_" + 2 * i)));
117+
IntervalsSource source2 = Intervals.maxgaps(0, Intervals.ordered(Intervals.term("term_" + (2 * i + 1))));
118+
sources.add(Intervals.or(source1, source2));
119+
}
120+
121+
assertFalse(IntervalBuilder.canCombineSources(sources));
122+
}
123+
124+
public void testCanCombineSourcesSuccess() {
125+
List<IntervalsSource> sources = new ArrayList<>();
126+
127+
for (int i = 0; i < 10; i++) {
128+
IntervalsSource source1 = Intervals.maxgaps(0, Intervals.ordered(Intervals.term("term_" + 2 * i)));
129+
IntervalsSource source2 = Intervals.maxgaps(0, Intervals.ordered(Intervals.term("term_" + (2 * i + 1))));
130+
sources.add(Intervals.or(source1, source2));
131+
}
132+
133+
assertTrue(IntervalBuilder.canCombineSources(sources));
134+
}
107135
}

test/framework/src/main/java/org/opensearch/test/AbstractQueryTestCase.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,13 @@ public void testToQuery() throws IOException {
448448
* We do it this way in SearchService where
449449
* we first rewrite the query with a private context, then reset the context and then build the actual lucene query*/
450450
QueryBuilder rewritten = rewriteQuery(firstQuery, new QueryShardContext(context));
451-
Query firstLuceneQuery = rewritten.toQuery(context);
451+
Query firstLuceneQuery;
452+
try {
453+
firstLuceneQuery = rewritten.toQuery(context);
454+
} catch (IllegalArgumentException e) {
455+
assertEquals("Too many disjunctions to expand", e.getMessage());
456+
continue;
457+
}
452458
assertNotNull("toQuery should not return null", firstLuceneQuery);
453459
assertLuceneQuery(firstQuery, firstLuceneQuery, context);
454460
// remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well

0 commit comments

Comments
 (0)