Skip to content

Commit

Permalink
Early terminate the slices for concurrent search if CircuitBreakingEx…
Browse files Browse the repository at this point in the history
…ception is thrown

Signed-off-by: Neetika Singhal <[email protected]>
  • Loading branch information
neetikasinghal committed Jan 3, 2024
1 parent 16d457d commit 88eefb9
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.lease.Releasable;
import org.opensearch.common.lucene.search.TopDocsAndMaxScore;
import org.opensearch.core.common.breaker.CircuitBreakingException;
import org.opensearch.search.DocValueFormat;
import org.opensearch.search.SearchService;
import org.opensearch.search.dfs.AggregatedDfs;
Expand Down Expand Up @@ -269,6 +270,11 @@ public void search(

@Override
protected void search(List<LeafReaderContext> leaves, Weight weight, Collector collector) throws IOException {

if (searchContext.isCircuitBreakerTripped()) {
return;

Check warning on line 275 in server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java#L275

Added line #L275 was not covered by tests
}

// Time series based workload by default traverses segments in desc order i.e. latest to the oldest order.
// This is actually beneficial for search queries to start search on latest segments first for time series workload.
// That can slow down ASC order queries on timestamp workload. So to avoid that slowdown, we will reverse leaf
Expand Down Expand Up @@ -297,6 +303,9 @@ private void searchLeaf(LeafReaderContext ctx, Weight weight, Collector collecto
if (canMatch(ctx) == false) {
return;
}
if (searchContext.isCircuitBreakerTripped()) {
return;

Check warning on line 307 in server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java#L307

Added line #L307 was not covered by tests
}

final LeafCollector leafCollector;
try {
Expand All @@ -315,6 +324,9 @@ private void searchLeaf(LeafReaderContext ctx, Weight weight, Collector collecto
} catch (QueryPhase.TimeExceededException e) {
searchContext.setSearchTimedOut(true);
return;
} catch (CircuitBreakingException e) {
searchContext.setCircuitBreakerTripped(true);
return;

Check warning on line 329 in server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java#L327-L329

Added lines #L327 - L329 were not covered by tests
}
// catch early terminated exception and rethrow?
Bits liveDocs = ctx.reader().getLiveDocs();
Expand All @@ -330,6 +342,9 @@ private void searchLeaf(LeafReaderContext ctx, Weight weight, Collector collecto
} catch (QueryPhase.TimeExceededException e) {
searchContext.setSearchTimedOut(true);
return;
} catch (CircuitBreakingException e) {
searchContext.setCircuitBreakerTripped(true);
return;

Check warning on line 347 in server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java#L345-L347

Added lines #L345 - L347 were not covered by tests
}
}
} else {
Expand All @@ -349,6 +364,9 @@ private void searchLeaf(LeafReaderContext ctx, Weight weight, Collector collecto
} catch (QueryPhase.TimeExceededException e) {
searchContext.setSearchTimedOut(true);
return;
} catch (CircuitBreakingException e) {
searchContext.setCircuitBreakerTripped(true);
return;

Check warning on line 369 in server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java#L367-L369

Added lines #L367 - L369 were not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ public List<Aggregator> toAggregators(Collection<Collector> collectors) {
private InnerHitsContext innerHitsContext;

private volatile boolean searchTimedOut;
private volatile boolean circuitBreakerTripped;

protected SearchContext() {}

Expand All @@ -139,6 +140,14 @@ public void setSearchTimedOut(boolean searchTimedOut) {
this.searchTimedOut = searchTimedOut;
}

public boolean isCircuitBreakerTripped() {
return circuitBreakerTripped;
}

public void setCircuitBreakerTripped(boolean circuitBreakerTripped) {
this.circuitBreakerTripped = circuitBreakerTripped;
}

Check warning on line 149 in server/src/main/java/org/opensearch/search/internal/SearchContext.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/SearchContext.java#L148-L149

Added lines #L148 - L149 were not covered by tests

@Override
public final void close() {
if (closed.compareAndSet(false, true)) {
Expand Down

0 comments on commit 88eefb9

Please sign in to comment.