Adding cache eviction and listener for invalidating index field type … #142
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
…mappings on index deletion/update Signed-off-by: Ankit Jain <akjain@amazon.com>
Signed-off-by: Ankit Jain <akjain@amazon.com>
Signed-off-by: Ankit Jain <akjain@amazon.com>
Signed-off-by: Ankit Jain <akjain@amazon.com>
|
Existing tests still failing. Please take a look @jainankitk |
|
Looks like one of the reasons of the build failures is the license headers: |
ansjcy
reviewed
Oct 10, 2024
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Outdated
Show resolved
Hide resolved
ansjcy
approved these changes
Oct 10, 2024
There was a problem hiding this comment.
LGTM, left 1 minor comment.
@jainankitk do you think we should add the cache size as part of the healthcheck API as well?
Also we need to add the license header as caught by the build.
deshsidd
reviewed
Oct 10, 2024
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Outdated
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Outdated
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Outdated
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Show resolved
Hide resolved
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Show resolved
Hide resolved
src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGenerator.java
Show resolved
Hide resolved
src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGenerator.java
Show resolved
Hide resolved
deshsidd
reviewed
Oct 10, 2024
src/main/java/org/opensearch/plugin/insights/core/service/categorizer/QueryShapeGenerator.java
Show resolved
Hide resolved
|
if its quick can add unit tests to make sure cache is utilized and evicting properly? |
would be good to have this as part of the health check APIs in case we see performance issues and need to debug @ansjcy |
|
Thanks @jainankitk, LGTM overall. Only pending items seem to be:
|
Signed-off-by: Ankit Jain <akjain@amazon.com>
Signed-off-by: Ankit Jain <akjain@amazon.com>
Signed-off-by: Ankit Jain <akjain@amazon.com>
deshsidd
approved these changes
Oct 10, 2024
deshsidd
reviewed
Oct 10, 2024
...main/java/org/opensearch/plugin/insights/core/service/categorizer/IndicesFieldTypeCache.java
Show resolved
Hide resolved
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Oct 10, 2024
#142) * Adding cache eviction and listener for invalidating index field type mappings on index deletion/update Signed-off-by: Ankit Jain <akjain@amazon.com> * Fixing spotless violations Signed-off-by: Ankit Jain <akjain@amazon.com> * Fixing code to use .index instead of .findMappings Signed-off-by: Ankit Jain <akjain@amazon.com> * Fixing spotless violations Signed-off-by: Ankit Jain <akjain@amazon.com> * Addressing review comments Signed-off-by: Ankit Jain <akjain@amazon.com> * Fixing existing test failures Signed-off-by: Ankit Jain <akjain@amazon.com> * Fixing existing test failures Signed-off-by: Ankit Jain <akjain@amazon.com> --------- Signed-off-by: Ankit Jain <akjain@amazon.com> (cherry picked from commit fb24118) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
jainankitk
pushed a commit
that referenced
this pull request
Oct 10, 2024
#142) (#143) * Adding cache eviction and listener for invalidating index field type mappings on index deletion/update * Fixing spotless violations * Fixing code to use .index instead of .findMappings * Fixing spotless violations * Addressing review comments * Fixing existing test failures * Fixing existing test failures --------- (cherry picked from commit fb24118) Signed-off-by: Ankit Jain <akjain@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.
…mappings on index deletion/update
Description
This change primarily achieves 2 things:
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
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.