-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix StreamStringTermsAggregatorTests by replacing RandomIndexWriter with plain IndexWriter #18956 WIP #19404
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 StreamStringTermsAggregatorTests by replacing RandomIndexWriter with plain IndexWriter #18956 WIP #19404
Conversation
- I replaced RandomIndexWriter to plain IndexWriter to avoid unexpected segments in test which are unable to handle more than single segment - for other test I've added explicit segmenting. TODO It needs to be reviewed. - Perhaps StreamingTermsAggregator should reject second segment via assert or there should be a proper publish/reset flow. TODO, WIP so far this commit fixes the known reason for flackines (I belive) Signed-off-by: Mikhail Khludnev <[email protected]>
Got reproducible failure on the single segment. It doesn't comply with the current hypothesis.
|
❕ Gradle check result for b5cd364: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19404 +/- ##
============================================
+ Coverage 72.90% 72.93% +0.02%
+ Complexity 69915 69859 -56
============================================
Files 5675 5675
Lines 320841 320865 +24
Branches 46387 46388 +1
============================================
+ Hits 233909 234011 +102
+ Misses 68031 67843 -188
- Partials 18901 19011 +110 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@mkhludnev do you want to address TODO as part of different PR? If yes, we can probably merge this one |
Signed-off-by: Mikhail Khludnev <[email protected]>
fixed.
Right. We can treat this PR as urgent fix for reducing CI noize. |
Signed-off-by: Mikhail Khludnev <[email protected]>
❌ Gradle check result for bca5be7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Mikhail Khludnev <[email protected]>
attempt #19416 |
this PR left |
…ith plain IndexWriter opensearch-project#18956 WIP (opensearch-project#19404) * fix opensearch-project#18956 WIP - I replaced RandomIndexWriter to plain IndexWriter to avoid unexpected segments in test which are unable to handle more than single segment - for other test I've added explicit segmenting. TODO It needs to be reviewed. - Perhaps StreamingTermsAggregator should reject second segment via assert or there should be a proper publish/reset flow. TODO, WIP so far this commit fixes the known reason for flakiness (I believe) Signed-off-by: Mikhail Khludnev <[email protected]> * newIndexWriterConfig() causes segmenting via flushes by bufferedDocs Signed-off-by: Mikhail Khludnev <[email protected]> * subAggs test goes single segment Signed-off-by: Mikhail Khludnev <[email protected]> * asserting single segment Signed-off-by: Mikhail Khludnev <[email protected]> --------- Signed-off-by: Mikhail Khludnev <[email protected]>
so far this commit fixes the known reason for flackines (I belive)
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.