Skip to content

Commit 630b65b

Browse files
john-wagsterelasticsearchmachine
john-wagster
and
elasticsearchmachine
authored
[9.2] Address CompoundRetrieverBuilder Failure Handling (#136732) (#139483)
* Address CompoundRetrieverBuilder Failure Handling (#136732) * passing back message and status code from each failed shard instead * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
1 parent ef03007 commit 630b65b

File tree

16 files changed

+464
-39
lines changed

16 files changed

+464
-39
lines changed

‎docs/changelog/136732.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 136732
2+
summary: Added logic for individual shard failure handling for `CompoundRetrieverBuilder` and fixed how partial search results flag is passed through to `CompoundRetrieverBuilder`
3+
area: Search
4+
type: bug
5+
issues: []

‎server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchRedStateIndexIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public void testClusterDisallowPartialsWithRedState() throws Exception {
9595
}
9696

9797
private void setClusterDefaultAllowPartialResults(boolean allowPartialResults) {
98-
String key = SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS.getKey();
98+
String key = SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING.getKey();
9999

100100
Settings persistentSettings = Settings.builder().put(key, allowPartialResults).build();
101101

@@ -128,6 +128,6 @@ private void buildRedIndex(int numShards) throws Exception {
128128

129129
@After
130130
public void cleanup() throws Exception {
131-
updateClusterSettings(Settings.builder().putNull(SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS.getKey()));
131+
updateClusterSettings(Settings.builder().putNull(SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING.getKey()));
132132
}
133133
}

‎server/src/main/java/org/elasticsearch/action/search/SearchRequest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Objects;
4545

4646
import static org.elasticsearch.action.ValidateActions.addValidationError;
47+
import static org.elasticsearch.search.SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS;
4748

4849
/**
4950
* A request to execute search against one or more indices (or all).
@@ -325,7 +326,10 @@ public void writeTo(StreamOutput out, boolean skipIndices) throws IOException {
325326
public ActionRequestValidationException validate() {
326327
ActionRequestValidationException validationException = null;
327328
boolean scroll = scroll() != null;
328-
boolean allowPartialSearchResults = allowPartialSearchResults() != null && allowPartialSearchResults();
329+
330+
boolean allowPartialSearchResults = allowPartialSearchResults() == null
331+
? DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
332+
: allowPartialSearchResults();
329333

330334
if (source != null) {
331335
validationException = source.validate(validationException, scroll, allowPartialSearchResults);

‎server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,9 @@ public void onFailure(Exception e) {
584584
});
585585

586586
final boolean isExplain = source != null && source.explain() != null && source.explain();
587+
final boolean allowPartialSearchResults = original.allowPartialSearchResults() != null
588+
? original.allowPartialSearchResults()
589+
: searchService.defaultAllowPartialSearchResults();
587590
Rewriteable.rewriteAndFetch(
588591
original,
589592
searchService.getRewriteContext(
@@ -593,7 +596,8 @@ public void onFailure(Exception e) {
593596
resolvedIndices,
594597
original.pointInTimeBuilder(),
595598
shouldMinimizeRoundtrips(original),
596-
isExplain
599+
isExplain,
600+
allowPartialSearchResults
597601
),
598602
rewriteListener
599603
);

‎server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ public void apply(Settings value, Settings current, Settings previous) {
362362
MasterService.MASTER_SERVICE_SLOW_TASK_LOGGING_THRESHOLD_SETTING,
363363
MasterService.MASTER_SERVICE_STARVATION_LOGGING_THRESHOLD_SETTING,
364364
SearchService.DEFAULT_SEARCH_TIMEOUT_SETTING,
365-
SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS,
365+
SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING,
366366
TransportSearchAction.SHARD_COUNT_LIMIT_SETTING,
367367
TransportSearchAction.DEFAULT_PRE_FILTER_SHARD_SIZE,
368368
RemoteClusterSettings.REMOTE_CLUSTER_SKIP_UNAVAILABLE,

‎server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java‎

Lines changed: 101 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
import java.util.function.Predicate;
5151
import java.util.stream.Collectors;
5252

53+
import static org.elasticsearch.search.SearchService.DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS;
54+
5355
/**
5456
* Context object used to rewrite {@link QueryBuilder} instances into simplified version.
5557
*/
@@ -78,6 +80,7 @@ public class QueryRewriteContext {
7880
private QueryRewriteInterceptor queryRewriteInterceptor;
7981
private final Boolean ccsMinimizeRoundTrips;
8082
private final boolean isExplain;
83+
private final boolean allowPartialSearchResults;
8184

8285
public QueryRewriteContext(
8386
final XContentParserConfiguration parserConfiguration,
@@ -99,9 +102,9 @@ public QueryRewriteContext(
99102
final PointInTimeBuilder pit,
100103
final QueryRewriteInterceptor queryRewriteInterceptor,
101104
final Boolean ccsMinimizeRoundTrips,
102-
final boolean isExplain
105+
final boolean isExplain,
106+
final boolean allowPartialSearchResults
103107
) {
104-
105108
this.parserConfiguration = parserConfiguration;
106109
this.client = client;
107110
this.nowInMillis = nowInMillis;
@@ -123,6 +126,54 @@ public QueryRewriteContext(
123126
this.queryRewriteInterceptor = queryRewriteInterceptor;
124127
this.ccsMinimizeRoundTrips = ccsMinimizeRoundTrips;
125128
this.isExplain = isExplain;
129+
this.allowPartialSearchResults = allowPartialSearchResults;
130+
}
131+
132+
public QueryRewriteContext(
133+
final XContentParserConfiguration parserConfiguration,
134+
final Client client,
135+
final LongSupplier nowInMillis,
136+
final MapperService mapperService,
137+
final MappingLookup mappingLookup,
138+
final Map<String, MappedFieldType> runtimeMappings,
139+
final IndexSettings indexSettings,
140+
final TransportVersion minTransportVersion,
141+
final String localClusterAlias,
142+
final Index fullyQualifiedIndex,
143+
final Predicate<String> indexNameMatcher,
144+
final NamedWriteableRegistry namedWriteableRegistry,
145+
final ValuesSourceRegistry valuesSourceRegistry,
146+
final BooleanSupplier allowExpensiveQueries,
147+
final ScriptCompiler scriptService,
148+
final ResolvedIndices resolvedIndices,
149+
final PointInTimeBuilder pit,
150+
final QueryRewriteInterceptor queryRewriteInterceptor,
151+
final Boolean ccsMinimizeRoundTrips,
152+
final boolean isExplain
153+
) {
154+
this(
155+
parserConfiguration,
156+
client,
157+
nowInMillis,
158+
mapperService,
159+
Objects.requireNonNull(mappingLookup),
160+
runtimeMappings,
161+
indexSettings,
162+
minTransportVersion,
163+
localClusterAlias,
164+
fullyQualifiedIndex,
165+
indexNameMatcher,
166+
namedWriteableRegistry,
167+
valuesSourceRegistry,
168+
allowExpensiveQueries,
169+
scriptService,
170+
resolvedIndices,
171+
pit,
172+
queryRewriteInterceptor,
173+
ccsMinimizeRoundTrips,
174+
isExplain,
175+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
176+
);
126177
}
127178

128179
public QueryRewriteContext(final XContentParserConfiguration parserConfiguration, final Client client, final LongSupplier nowInMillis) {
@@ -146,7 +197,8 @@ public QueryRewriteContext(final XContentParserConfiguration parserConfiguration
146197
null,
147198
null,
148199
null,
149-
false
200+
false,
201+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
150202
);
151203
}
152204

@@ -171,7 +223,46 @@ public QueryRewriteContext(
171223
pit,
172224
queryRewriteInterceptor,
173225
ccsMinimizeRoundTrips,
174-
false
226+
false,
227+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
228+
);
229+
}
230+
231+
public QueryRewriteContext(
232+
final XContentParserConfiguration parserConfiguration,
233+
final Client client,
234+
final LongSupplier nowInMillis,
235+
final TransportVersion minTransportVersion,
236+
final String localClusterAlias,
237+
final ResolvedIndices resolvedIndices,
238+
final PointInTimeBuilder pit,
239+
final QueryRewriteInterceptor queryRewriteInterceptor,
240+
final Boolean ccsMinimizeRoundTrips,
241+
final boolean isExplain,
242+
final boolean allowPartialSearchResults
243+
) {
244+
this(
245+
parserConfiguration,
246+
client,
247+
nowInMillis,
248+
null,
249+
MappingLookup.EMPTY,
250+
Collections.emptyMap(),
251+
null,
252+
minTransportVersion,
253+
localClusterAlias,
254+
null,
255+
null,
256+
null,
257+
null,
258+
null,
259+
null,
260+
resolvedIndices,
261+
pit,
262+
queryRewriteInterceptor,
263+
ccsMinimizeRoundTrips,
264+
isExplain,
265+
allowPartialSearchResults
175266
);
176267
}
177268

@@ -207,7 +298,8 @@ public QueryRewriteContext(
207298
pit,
208299
queryRewriteInterceptor,
209300
ccsMinimizeRoundTrips,
210-
isExplain
301+
isExplain,
302+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
211303
);
212304
}
213305

@@ -324,6 +416,10 @@ public boolean isExplain() {
324416
return this.isExplain;
325417
}
326418

419+
public boolean allowPartialSearchResults() {
420+
return this.allowPartialSearchResults;
421+
}
422+
327423
public NamedWriteableRegistry getWriteableRegistry() {
328424
return writeableRegistry;
329425
}

‎server/src/main/java/org/elasticsearch/indices/IndicesService.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,7 +1861,8 @@ public QueryRewriteContext getRewriteContext(
18611861
ResolvedIndices resolvedIndices,
18621862
PointInTimeBuilder pit,
18631863
final Boolean ccsMinimizeRoundTrips,
1864-
final boolean isExplain
1864+
final boolean isExplain,
1865+
final boolean allowPartialSearchResults
18651866
) {
18661867
return new QueryRewriteContext(
18671868
parserConfig,
@@ -1873,7 +1874,8 @@ public QueryRewriteContext getRewriteContext(
18731874
pit,
18741875
queryRewriteInterceptor,
18751876
ccsMinimizeRoundTrips,
1876-
isExplain
1877+
isExplain,
1878+
allowPartialSearchResults
18771879
);
18781880
}
18791881

‎server/src/main/java/org/elasticsearch/search/SearchService.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,11 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
221221
Property.Dynamic,
222222
Property.NodeScope
223223
);
224-
public static final Setting<Boolean> DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS = Setting.boolSetting(
224+
225+
public static final boolean DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS = true;
226+
public static final Setting<Boolean> DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING = Setting.boolSetting(
225227
"search.default_allow_partial_results",
226-
true,
228+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS,
227229
Property.Dynamic,
228230
Property.NodeScope
229231
);
@@ -411,9 +413,9 @@ public SearchService(
411413

412414
minimumDocsPerSlice = MINIMUM_DOCS_PER_SLICE.get(settings);
413415

414-
defaultAllowPartialSearchResults = DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS.get(settings);
416+
defaultAllowPartialSearchResults = DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING.get(settings);
415417
clusterService.getClusterSettings()
416-
.addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, this::setDefaultAllowPartialSearchResults);
418+
.addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS_SETTING, this::setDefaultAllowPartialSearchResults);
417419

418420
maxOpenScrollContext = MAX_OPEN_SCROLL_CONTEXT.get(settings);
419421
clusterService.getClusterSettings().addSettingsUpdateConsumer(MAX_OPEN_SCROLL_CONTEXT, this::setMaxOpenScrollContext);
@@ -2142,7 +2144,16 @@ public QueryRewriteContext getRewriteContext(
21422144
PointInTimeBuilder pit,
21432145
final Boolean ccsMinimizeRoundTrips
21442146
) {
2145-
return getRewriteContext(nowInMillis, minTransportVersion, clusterAlias, resolvedIndices, pit, ccsMinimizeRoundTrips, false);
2147+
return getRewriteContext(
2148+
nowInMillis,
2149+
minTransportVersion,
2150+
clusterAlias,
2151+
resolvedIndices,
2152+
pit,
2153+
ccsMinimizeRoundTrips,
2154+
false,
2155+
DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS
2156+
);
21462157
}
21472158

21482159
/**
@@ -2155,7 +2166,8 @@ public QueryRewriteContext getRewriteContext(
21552166
ResolvedIndices resolvedIndices,
21562167
PointInTimeBuilder pit,
21572168
final Boolean ccsMinimizeRoundTrips,
2158-
final boolean isExplain
2169+
final boolean isExplain,
2170+
final boolean allowPartialSearchResults
21592171
) {
21602172
return indicesService.getRewriteContext(
21612173
nowInMillis,
@@ -2164,7 +2176,8 @@ public QueryRewriteContext getRewriteContext(
21642176
resolvedIndices,
21652177
pit,
21662178
ccsMinimizeRoundTrips,
2167-
isExplain
2179+
isExplain,
2180+
allowPartialSearchResults
21682181
);
21692182
}
21702183

‎server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.action.search.MultiSearchResponse;
2020
import org.elasticsearch.action.search.SearchRequest;
2121
import org.elasticsearch.action.search.SearchResponse;
22+
import org.elasticsearch.action.search.ShardSearchFailure;
2223
import org.elasticsearch.action.search.TransportMultiSearchAction;
2324
import org.elasticsearch.features.NodeFeature;
2425
import org.elasticsearch.index.query.QueryBuilder;
@@ -174,11 +175,17 @@ public void onResponse(MultiSearchResponse items) {
174175
}
175176
} else {
176177
assert item.getResponse() != null;
177-
var rankDocs = getRankDocs(item.getResponse());
178-
innerRetrievers.get(i).retriever().setRankDocs(rankDocs);
179-
topDocs.add(rankDocs);
178+
// TODO: handle partial failures by passing them back up with the results
179+
if (item.getResponse().getFailedShards() > 0 && ctx.allowPartialSearchResults() == false) {
180+
statusCode = handleShardFailures(item.getResponse(), statusCode, failures);
181+
} else {
182+
var rankDocs = getRankDocs(item.getResponse());
183+
innerRetrievers.get(i).retriever().setRankDocs(rankDocs);
184+
topDocs.add(rankDocs);
185+
}
180186
}
181187
}
188+
182189
if (false == failures.isEmpty()) {
183190
assert statusCode != RestStatus.OK.getStatus();
184191
final String errMessage = "["
@@ -212,6 +219,37 @@ public void onFailure(Exception e) {
212219
return rankDocsRetrieverBuilder;
213220
}
214221

222+
int handleShardFailures(SearchResponse response, int statusCode, List<Exception> failures) {
223+
ShardSearchFailure[] shardFailures = response.getShardFailures();
224+
for (ShardSearchFailure shardFailure : shardFailures) {
225+
if (shardFailure != null) {
226+
int shardFailureStatusCode = ExceptionsHelper.status(shardFailure.getCause()).getStatus();
227+
failures.add(
228+
processInnerItemFailureException(
229+
new ElasticsearchStatusException(
230+
"failed to retrieve data from shard ["
231+
+ shardFailure.shardId()
232+
+ "] with message: "
233+
+ shardFailure.getCause().getMessage(),
234+
RestStatus.fromCode(shardFailureStatusCode)
235+
)
236+
)
237+
);
238+
statusCode = Math.max(shardFailureStatusCode, statusCode);
239+
}
240+
}
241+
return statusCode;
242+
}
243+
244+
/**
245+
* Overridable method to check or modify any failures from child retrievers if needed
246+
* @param ex the exception thrown
247+
* @return the failure exception
248+
*/
249+
protected Exception processInnerItemFailureException(Exception ex) {
250+
return ex;
251+
}
252+
215253
@Override
216254
public final QueryBuilder topDocsQuery() {
217255
throw new IllegalStateException("Should not be called, missing a rewrite?");
@@ -249,12 +287,6 @@ public ActionRequestValidationException validate(
249287
validationException
250288
);
251289
}
252-
if (allowPartialSearchResults) {
253-
validationException = addValidationError(
254-
"cannot specify [" + getName() + "] and [allow_partial_search_results]",
255-
validationException
256-
);
257-
}
258290
if (isScroll) {
259291
validationException = addValidationError("cannot specify [" + getName() + "] and [scroll]", validationException);
260292
}

0 commit comments

Comments
 (0)