Skip to content

Commit 476eeb8

Browse files
committed
Fix assertion error search reduce errors
When receiving exceptions from more than one node on data node search reduction, SearchQueryThenFetchAsyncAction will raise more than one phase failure. This can lead to calling the listener in AbstractSearchAsyncAction more than once, which in turn in tests trips an assertion in ActionListener#assertFirstRun.
1 parent 0b06d9f commit 476eeb8

File tree

3 files changed

+31
-0
lines changed

3 files changed

+31
-0
lines changed

server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,6 +2145,31 @@ public void testLongSortOptimizationCorrectResults() {
21452145
);
21462146
}
21472147

2148+
public void testSortMixedFieldTypesSeveralDocs() {
2149+
assertAcked(
2150+
prepareCreate("index_long").setMapping("foo", "type=long"),
2151+
prepareCreate("index_double").setMapping("foo", "type=double")
2152+
);
2153+
2154+
List<IndexRequestBuilder> builders = new ArrayList<>();
2155+
2156+
for (int i = 0; i < 10; i++) {
2157+
builders.add(prepareIndex("index_long").setId(String.valueOf(i)).setSource("foo", i));
2158+
builders.add(prepareIndex("index_double").setId(String.valueOf(i)).setSource("foo", i));
2159+
}
2160+
indexRandom(true, false, builders);
2161+
2162+
String errMsg = "Can't sort on field [foo]; the field has incompatible sort types";
2163+
2164+
{ // mixing long and double types is not allowed
2165+
SearchPhaseExecutionException exc = expectThrows(
2166+
SearchPhaseExecutionException.class,
2167+
prepareSearch("index_long", "index_double").addSort(new FieldSortBuilder("foo")).setSize(20)
2168+
);
2169+
assertThat(exc.getCause().toString(), containsString(errMsg));
2170+
}
2171+
}
2172+
21482173
public void testSortMixedFieldTypes() {
21492174
assertAcked(
21502175
prepareCreate("index_long").setMapping("foo", "type=long"),

server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ abstract class AbstractSearchAsyncAction<Result extends SearchPhaseResult> exten
9393
private final Map<String, PendingExecutions> pendingExecutionsPerNode;
9494
private final AtomicBoolean requestCancelled = new AtomicBoolean();
9595
private final int skippedCount;
96+
private final AtomicBoolean phaseFailureEncountered = new AtomicBoolean();
9697

9798
// protected for tests
9899
protected final SubscribableListener<Void> doneFuture = new SubscribableListener<>();
@@ -621,6 +622,10 @@ protected BytesReference buildSearchContextId(ShardSearchFailure[] failures) {
621622
* @param cause the cause of the phase failure
622623
*/
623624
public void onPhaseFailure(String phase, String msg, Throwable cause) {
625+
if (phaseFailureEncountered.compareAndSet(false, true) == false) {
626+
// we already encountered a phase failure, so we ignore this one
627+
return;
628+
}
624629
raisePhaseFailure(new SearchPhaseExecutionException(phase, msg, cause, buildShardFailures()));
625630
}
626631

server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@ public void handleException(TransportException e) {
521521
if (results instanceof QueryPhaseResultConsumer queryPhaseResultConsumer) {
522522
queryPhaseResultConsumer.failure.compareAndSet(null, cause);
523523
}
524+
logger.debug("Raising phase failure for " + cause + " while executing search on node " + routing.nodeId());
524525
onPhaseFailure(getName(), "", cause);
525526
}
526527
}

0 commit comments

Comments
 (0)