-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix SuggestSearch.testSkipDuplicates by forceing refresh when indexing its test documents #11068
Fix SuggestSearch.testSkipDuplicates by forceing refresh when indexing its test documents #11068
Conversation
During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning. As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting.
Signed-off-by: Peter Nied <petern@amazon.com>
Gradle Check (Jenkins) Run Completed with:
|
Compatibility status:Checks if related components are compatible with change 5d128ee Incompatible componentsIncompatible components: [https://github.com/opensearch-project/performance-analyzer.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/k-nn.git] |
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
@@ Coverage Diff @@
## main #11068 +/- ##
============================================
- Coverage 71.29% 71.22% -0.07%
- Complexity 58742 58781 +39
============================================
Files 4872 4872
Lines 276777 276780 +3
Branches 40240 40241 +1
============================================
- Hits 197316 197138 -178
- Misses 62943 63236 +293
+ Partials 16518 16406 -112
|
|
...er/src/internalClusterTest/java/org/opensearch/search/suggest/CompletionSuggestSearchIT.java
Show resolved
Hide resolved
@peternied I guess we don't need to merge the logging change and can be done locally to debug the test issue by running with Repeat annotation. Or is the refresh fixing the test here and logs are added to debug future failure just in case if the issue happens again ? |
@sohami I haven't been able to reproduce the issue locally. I've run ~2000 iterations on this test with and without difference seed / locales. I've tweaked the 'random' elements to be hardcode to attempt to find an issue - no luck. The only theory I have is around the index not being fully current when the search query is issued. I think its of minimal risk to force refreshing on index. By leaving telemetry in, if this comes back up I should have more precise information about what wasn't working to better root cause it. How does that sound to you? |
@Poojita-Raj @mch2 @reta @dreamer-89 could I get a review on this change? Added you folks since you reported this test as a failure in the past 2 days. |
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.
just nits so approved
...er/src/internalClusterTest/java/org/opensearch/search/suggest/CompletionSuggestSearchIT.java
Show resolved
Hide resolved
Signed-off-by: Peter Nied <petern@amazon.com>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-11068-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0ba5d5807860a799c76f0fb6e063e8c5bd458492
# Push it to GitHub
git push --set-upstream origin backport/backport-11068-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
Gradle Check (Jenkins) Run Completed with:
|
…sh when indexing its test documents (opensearch-project#11068) During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning. As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit 0ba5d58)
…sh when indexing its test documents (#11068) (#11141) During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning. As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit 0ba5d58) Signed-off-by: Peter Nied <petern@amazon.com>
…g its test documents (opensearch-project#11068) During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning. As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting. Signed-off-by: Peter Nied <petern@amazon.com>
…g its test documents (opensearch-project#11068) During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning. As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting. Signed-off-by: Peter Nied <petern@amazon.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Description
During the testSkipDuplicates its possible that not all documents were fully indexed by the time the search with suggestions was created, updating the indexing operations to refresh the index before returning.
As its possible that did not fix the issue, I've added logging around the test case to capture the state when the error occurred that can assist in future troubleshooting.
Related Issues
Check List
New functionality has been documented.New functionality has javadoc addedPublic documentation issue/PR createdBy 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.