Skip to content

Commit

Permalink
Ignore cancellation error when search is cancelled (#64240)
Browse files Browse the repository at this point in the history
Since #63520, we will cancel a search that hits shard failures and does 
not accept partial results. However, that change can return the wrong
HTTP code for bad requests (from 4xx to 5xx) due to the cancellation.

Relates #63520
Closes #64012
Closes #63702
  • Loading branch information
dnhatn committed Oct 27, 2020
1 parent 7384e68 commit 4388449
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.elasticsearch.search.internal.InternalSearchResponse;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.internal.ShardSearchRequest;
import org.elasticsearch.tasks.TaskCancelledException;
import org.elasticsearch.transport.Transport;

import java.util.ArrayDeque;
Expand Down Expand Up @@ -437,8 +438,9 @@ protected void onShardGroupFailure(int shardIndex, SearchShardTarget shardTarget
*/
@Override
public final void onShardFailure(final int shardIndex, @Nullable SearchShardTarget shardTarget, Exception e) {
// we don't aggregate shard failures on non active shards (but do keep the header counts right)
if (TransportActions.isShardNotAvailableException(e) == false) {
// we don't aggregate shard failures on non active shards and failures due to the internal cancellation,
// but do keep the header counts right
if (TransportActions.isShardNotAvailableException(e) == false && (requestCancelled.get() && isTaskCancelledException(e)) == false) {
AtomicArray<ShardSearchFailure> shardFailures = this.shardFailures.get();
// lazily create shard failures, so we can early build the empty shard failure list in most cases (no failures)
if (shardFailures == null) { // this is double checked locking but it's fine since SetOnce uses a volatile read internally
Expand Down Expand Up @@ -469,6 +471,10 @@ public final void onShardFailure(final int shardIndex, @Nullable SearchShardTarg
results.consumeShardFailure(shardIndex);
}

private static boolean isTaskCancelledException(Exception e) {
return ExceptionsHelper.unwrapCausesAndSuppressed(e, ex -> ex instanceof TaskCancelledException).isPresent();
}

/**
* Executed once for every successful shard level request.
* @param result the result returned form the shard
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,6 @@ public void testRemoveAsyncIndex() throws Exception {
ensureTaskRemoval(newResp.getId());
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/63702")
public void testSearchPhaseFailureNoCause() throws Exception {
SubmitAsyncSearchRequest request = new SubmitAsyncSearchRequest(indexName);
request.setKeepOnCompletion(true);
Expand Down

0 comments on commit 4388449

Please sign in to comment.