Implement basic search with point in time and search after #2847
Conversation
graytaylor0
requested review from
chenqi0805,
engechas,
dinujoh,
kkondaka,
cmanning09,
asifsmohammed,
dlvenable and
oeyh
as code owners
graytaylor0
force-pushed
the
PitSearchImpl
branch
from
67f1df6 to
becb43d
Compare
Signed-off-by: Taylor Gray <[email protected]>
graytaylor0
force-pushed
the
PitSearchImpl
branch
from
becb43d to
a858cee
Compare
|
|
||
| import java.util.List; | ||
|
|
||
| public class SearchPointInTimeResponse { |
There was a problem hiding this comment.
I'd perhaps rename this. At first glance it looks like it is intended to be similar to the opensearch-java APIs. But, it is quite different.
Perhaps: SearchPointInTimeResults or SearchPointInTimeOutput?
|
|
||
| return SearchPointInTimeResponse.builder() | ||
| .withDocuments(documents) | ||
| .withNextSearchAfter(searchResponse.hits().hits().get(searchResponse.hits().hits().size() - 1).sort()) |
There was a problem hiding this comment.
I'm not sure I follow how you get the next search after. Is there no order from the OpenSearch response that could help here?
Also, you have an inline sort(). That should probably be pulled out of this line as it is somewhat signficiant.
There was a problem hiding this comment.
I see, this is the OpenSearch "sort" model, not a collection sort.
|
|
||
| return SearchPointInTimeResponse.builder() | ||
| .withDocuments(documents) | ||
| .withNextSearchAfter(searchResponse.hits().hits().get(searchResponse.hits().hits().size() - 1).sort()) |
There was a problem hiding this comment.
Can the size be 0? What if your paging results in a final empty page?
There was a problem hiding this comment.
Yes it can be good catch.
| @@ -20,10 +20,10 @@ public class SearchConfiguration { | |||
| private static final Logger LOG = LoggerFactory.getLogger(SearchConfiguration.class); | |||
|
|
|||
| @JsonProperty("batch_size") | |||
| private Integer batchSize; | |||
| private Integer batchSize = 1000; | |||
There was a problem hiding this comment.
nit
DEFAULT_BATCH_SIZE=1000
Signed-off-by: Taylor Gray <[email protected]>
|
I'd also recommend that you rename |
…h-project#2847) Implement basic search with point in time and search after Signed-off-by: Taylor Gray <[email protected]> Signed-off-by: Marcos_Gonzalez_Mayedo <[email protected]>
Description
Implements searching via point in time. Paginates based on the
batch_sizein the opensearch config (defaults to 1000)Limitations of this PR:
querypassed in the config is currently unused, as the opensearch java client does not support passing query strings or query maps directly to search requests ([FEATURE] Send search request query DSL json directly to search request opensearch-java#525). For now, only processing all documents via amatch_allquery and default sorting in ascending mode is supported.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.