-
Notifications
You must be signed in to change notification settings - Fork 25.5k
CPS search should not use skip_unavailable
#132927
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CPS search should not use skip_unavailable
#132927
Conversation
0550b04
to
8de8f1f
Compare
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
...rc/internalClusterTest/java/org/elasticsearch/search/ccs/CpsDoesNotUseSkipUnavailableIT.java
Outdated
Show resolved
Hide resolved
|
||
private SearchRequest getSearchRequest(boolean allowPartialResults) { | ||
// Include both origin and linked cluster in the search op. | ||
var searchRequest = new SearchRequest("*", "*:*"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just *:*
or even cluster-a:*
? I understand only the remote part is being tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no specific reason. It's just an assurance that the search proceeds fine -- local cluster is marked as successful and the other is either skipped or there's a fatal error.
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java
Outdated
Show resolved
Hide resolved
...rc/internalClusterTest/java/org/elasticsearch/search/ccs/CpsDoesNotUseSkipUnavailableIT.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Good solution.
…improv * upstream/main: (58 commits) Fixing flaky LoggedExec (tests) (elastic#133215) CPS search should not use `skip_unavailable` (elastic#132927) Don't fail search if bottom doc can't be formatted (elastic#133188) Mute org.elasticsearch.xpack.esql.action.RandomizedTimeSeriesIT testGroupByNothing elastic#133225 Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedSetDocValuesWithSkipperSmall elastic#133224 Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedNumberMergeAwayAllValuesWithSkipper elastic#133223 Adding support for index.number_of_replicas to data stream settings (elastic#132748) Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedDocValuesSingleUniqueValue elastic#133221 Fix VectorSimilarityFunctionsIT (elastic#133206) Mute org.elasticsearch.xpack.esql.action.RandomizedTimeSeriesIT testGroupBySubset elastic#133220 Increase the number of FORK branches in ForkGenerator (elastic#132019) Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=STORED]} elastic#133218 Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=DOC_VALUES]} elastic#133217 Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=NONE]} elastic#133216 Set default processor allocation for test clusters (elastic#133204) Add mapper for exponential histograms (elastic#132493) Fix offset handling in Murmur3Hasher (elastic#133193) unmute testDoesNotResolveClosedIndex (elastic#133115) Fix an AWS SDK v2 release note (elastic#133155) Limit the depth of a filter (elastic#133113) ...
* upstream/main: (58 commits) Fixing flaky LoggedExec (tests) (elastic#133215) CPS search should not use `skip_unavailable` (elastic#132927) Don't fail search if bottom doc can't be formatted (elastic#133188) Mute org.elasticsearch.xpack.esql.action.RandomizedTimeSeriesIT testGroupByNothing elastic#133225 Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedSetDocValuesWithSkipperSmall elastic#133224 Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedNumberMergeAwayAllValuesWithSkipper elastic#133223 Adding support for index.number_of_replicas to data stream settings (elastic#132748) Mute org.elasticsearch.index.codec.tsdb.es819.ES819TSDBDocValuesFormatTests testSortedDocValuesSingleUniqueValue elastic#133221 Fix VectorSimilarityFunctionsIT (elastic#133206) Mute org.elasticsearch.xpack.esql.action.RandomizedTimeSeriesIT testGroupBySubset elastic#133220 Increase the number of FORK branches in ForkGenerator (elastic#132019) Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=STORED]} elastic#133218 Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=DOC_VALUES]} elastic#133217 Mute org.elasticsearch.index.mapper.blockloader.IpFieldBlockLoaderTests testBlockLoader {preference=Params[syntheticSource=true, preference=NONE]} elastic#133216 Set default processor allocation for test clusters (elastic#133204) Add mapper for exponential histograms (elastic#132493) Fix offset handling in Murmur3Hasher (elastic#133193) unmute testDoesNotResolveClosedIndex (elastic#133115) Fix an AWS SDK v2 release note (elastic#133155) Limit the depth of a filter (elastic#133113) ...
For CPS S2D4, we'd not want CPS search to use
skip_unavailable
. With this PR, the skippable-logic instead is tucked away and a cluster is marked as skippable based on:skip_unavailable
(for non-CPS search).