Implement NoSearchContextWorker to search with search_after and not use pit or scroll, allow override with search_context_type parameter #2873
Conversation
graytaylor0
requested review from
chenqi0805,
engechas,
dinujoh,
kkondaka,
cmanning09,
asifsmohammed,
dlvenable and
oeyh
as code owners
| return null; | ||
| } | ||
|
|
||
| if (Objects.isNull(searchWithSearchAfterResults) && Objects.nonNull(openSearchIndexProgressState.getSearchAfter())) { |
There was a problem hiding this comment.
You should combine these two if statements otherwise it is little confusing.
There was a problem hiding this comment.
I don't understand what is confusing about it.
There was a problem hiding this comment.
It is not clear from the code that line at 148 searchWithSearchAfterResults is not null. better way of writing would be
if (Objects.isNull(searchWithSearchAfterResults)) {
if (Objects.nonNull(openSearchIndexProgressState.getSearchAfter())( {
return openSearchIndexProgressState.getSearchAfter();
} else {
return null;
}
}
This way we know after the code after the "if" block is the "else" block.
…se pit or scroll, allow override with search_context_type parameter Signed-off-by: Taylor Gray <tylgry@amazon.com>
graytaylor0
force-pushed
the
OpenSearchScroll
branch
from
bda4ac7
to
114f7db
Compare
Signed-off-by: Taylor Gray <tylgry@amazon.com>
| import java.util.Map; | ||
|
|
||
| public class SearchConfiguration { | ||
|
|
||
| private static final ObjectMapper objectMapper = new ObjectMapper(); | ||
| private static final Logger LOG = LoggerFactory.getLogger(SearchConfiguration.class); | ||
|
|
||
| // TODO: Should we default this to NONE and remove the version lookup to determine scroll or point-in-time as the default behavior? |
There was a problem hiding this comment.
My thought is that this looks like a similar situation to s3.compression - we can set NONE as default and have "AUTO" as an explicit option to use the version-dependent method.
There was a problem hiding this comment.
That's definitely a consideration. We can think about it more as this feature gets closer to release
…se pit or scroll, allow override with search_context_type parameter (opensearch-project#2873) * Implement NoSearchContextWorker to search with search_after and not use pit or scroll, allow override with search_context_type parameter Signed-off-by: Taylor Gray <tylgry@amazon.com> Signed-off-by: Marcos_Gonzalez_Mayedo <alemayed@amazon.com>
Description
NoSearchContextWorker, which will process indices using sort and search_after without creating search contexts via scroll or point in timesearch_options, thesearch_context_type.The
search_context_typecan be set tonone,point_in_time, orscroll. By default, point in time will be used for versions that can support it, and scroll will be used for versions that don't support point in time.However, there is a consideration to make the default just be
none, and just use the version lookup to validate that point in time can be used if thesearch_context_typeis set topoint_in_time.Issues Resolved
Related to #1985
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.