Search Rollup and Live Data Feature: Base case (No overlapping data) #898
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ronnaksaxena
requested review from
bowenlan-amzn,
getsaurabh02,
lezzago,
praveensameneni,
xluo-aws,
gaobinlong,
Hailong-am,
amsiglan,
sbcd90,
eirsep and
AWSHurneyt
as code owners
ronnaksaxena
changed the title
eirsep
reviewed
Aug 23, 2023
src/main/kotlin/org/opensearch/indexmanagement/IndexManagementPlugin.kt
Outdated
Show resolved
Hide resolved
eirsep
reviewed
Aug 23, 2023
src/main/kotlin/org/opensearch/indexmanagement/rollup/interceptor/ResponseInterceptor.kt
Outdated
Show resolved
Hide resolved
|
what do you mean by "Ignore exception being thrown by not calling validate indices in live data indices" in description? |
eirsep
reviewed
Aug 23, 2023
There was a problem hiding this comment.
thanks for making these changes, @ronnaksaxena
Added a few comments.
src/main/kotlin/org/opensearch/indexmanagement/rollup/interceptor/RollupInterceptor.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/org/opensearch/indexmanagement/rollup/interceptor/RollupInterceptor.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Ronnak Saxena <[email protected]>
Signed-off-by: Ronnak Saxena <[email protected]>
Signed-off-by: Ronnak Saxena <[email protected]>
deleted response interceptor init from indexManagementPlugin Signed-off-by: Ronnak Saxena <[email protected]>
deleted resonse interceptor from helper method Signed-off-by: Ronnak Saxena <[email protected]>
Will add response interceptor logic in next PR Signed-off-by: Ronnak Saxena <[email protected]>
uncommeneted rollup size check Signed-off-by: Ronnak Saxena <[email protected]>
Signed-off-by: Ronnak Saxena <[email protected]>
Comment on lines
+95
to
+101
| val concreteRolledUpIndexNames = mutableListOf<String>() | ||
| for (indexName in concreteIndices) { | ||
| if (isRollupIndex(indexName, clusterService.state())) { | ||
| concreteRolledUpIndexNames.add(indexName) | ||
| } | ||
| } | ||
| val filteredConcreteIndices = concreteRolledUpIndexNames.toTypedArray() |
There was a problem hiding this comment.
When the shard is a rollup index, you do a filtering and keep only the rollup indexes in the request. What about the other path, when the shard is a live index, do you also need a filtering?
Comment on lines
+1925
to
+1930
| var deleteResponse = client().makeRequest( | ||
| "POST", | ||
| "source_rollup_search/_delete_by_query", | ||
| mapOf("refresh" to "true"), | ||
| StringEntity("""{"query": {"match_all": {}}}""", ContentType.APPLICATION_JSON) | ||
| ) |
There was a problem hiding this comment.
If all the data from source index gets deleted, then it seems to me you are just search the rollup index later instead of both indexes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Handles the use case of of searching rollup indices and live indices that have no overlapping data between them
Base Case: No data overlap between rollup indices and live data indices
Changes:
CheckList:
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.