-
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
[Backport 2.x] Fix auto date histogram rounding assertion bug #17175
[Backport 2.x] Fix auto date histogram rounding assertion bug #17175
Conversation
{"run-benchmark-test": "id_5"} |
❌ Gradle check result for 640534d: 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? |
{"run-benchmark-test": "id_5"} |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #17175 +/- ##
==========================================
Coverage 71.98% 71.98%
+ Complexity 65987 65981 -6
==========================================
Files 5341 5341
Lines 307114 307157 +43
Branches 44824 44823 -1
==========================================
+ Hits 221068 221122 +54
+ Misses 67608 67501 -107
- Partials 18438 18534 +96 ☔ View full report in Codecov by Sentry. |
{"run-benchmark-test": "id_5"} |
Closing this for now to investigate performance regression: #17023 (comment) |
Local benchmarks don't replicate this, feel free to open and merge this. |
640534d
to
7fc3198
Compare
❌ Gradle check result for 7fc3198: 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? |
) * Add comments explanations for auto date histo increaseRoundingIfNeeded. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Add testFilterRewriteWithTZRoundingRangeAssert() to reproduce auto date histo assertion bug per opensearch-project#16932 Signed-off-by: Finn Carroll <carrofin@amazon.com> * Fix opensearch-project#16932. Ensure optimized path can only increase preparedRounding of agg. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Fast fail filter rewrite opt in data histo aggs for non UTC timezones Signed-off-by: Finn Carroll <carrofin@amazon.com> * Remove redundant UTC check from getInterval(). Signed-off-by: Finn Carroll <carrofin@amazon.com> * Save a call to prepareRounding if roundingIdx is unchanged. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Changelog Signed-off-by: Finn Carroll <carrofin@amazon.com> * Add ZoneId getter for date histo filter rewrite canOptimize check. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Disable ff optimzation for composite agg in canOptimize. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Handle utc timezone check Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com> * Remove redundant timeZone getter. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Simplify ff prepared rounding check. Signed-off-by: Finn Carroll <carrofin@amazon.com> --------- Signed-off-by: Finn Carroll <carrofin@amazon.com> Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> (cherry picked from commit de59264) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Finn Carroll <carrofin@amazon.com>
Signed-off-by: Finn Carroll <carrofin@amazon.com>
7fc3198
to
740ecab
Compare
* Fix auto date histogram rounding assertion bug (#17023) * Add comments explanations for auto date histo increaseRoundingIfNeeded. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Add testFilterRewriteWithTZRoundingRangeAssert() to reproduce auto date histo assertion bug per #16932 Signed-off-by: Finn Carroll <carrofin@amazon.com> * Fix #16932. Ensure optimized path can only increase preparedRounding of agg. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Fast fail filter rewrite opt in data histo aggs for non UTC timezones Signed-off-by: Finn Carroll <carrofin@amazon.com> * Remove redundant UTC check from getInterval(). Signed-off-by: Finn Carroll <carrofin@amazon.com> * Save a call to prepareRounding if roundingIdx is unchanged. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Changelog Signed-off-by: Finn Carroll <carrofin@amazon.com> * Add ZoneId getter for date histo filter rewrite canOptimize check. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Disable ff optimzation for composite agg in canOptimize. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Spotless apply Signed-off-by: Finn Carroll <carrofin@amazon.com> * Handle utc timezone check Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com> * Remove redundant timeZone getter. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Simplify ff prepared rounding check. Signed-off-by: Finn Carroll <carrofin@amazon.com> --------- Signed-off-by: Finn Carroll <carrofin@amazon.com> Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> (cherry picked from commit de59264) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Remove breaking abstract isUTC() getter from Rounding.java. Signed-off-by: Finn Carroll <carrofin@amazon.com> * Remove unused ZoneId getter. Signed-off-by: Finn Carroll <carrofin@amazon.com> --------- Signed-off-by: Finn Carroll <carrofin@amazon.com> Signed-off-by: bowenlan-amzn <bowenlan23@gmail.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> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> (cherry picked from commit a79c6e8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
#17211) * Fix auto date histogram rounding assertion bug (#17023) * Add comments explanations for auto date histo increaseRoundingIfNeeded. * Add testFilterRewriteWithTZRoundingRangeAssert() to reproduce auto date histo assertion bug per #16932 * Fix #16932. Ensure optimized path can only increase preparedRounding of agg. * Spotless apply * Fast fail filter rewrite opt in data histo aggs for non UTC timezones * Remove redundant UTC check from getInterval(). * Save a call to prepareRounding if roundingIdx is unchanged. * Spotless apply * Changelog * Add ZoneId getter for date histo filter rewrite canOptimize check. * Spotless apply * Disable ff optimzation for composite agg in canOptimize. * Spotless apply * Handle utc timezone check * Remove redundant timeZone getter. * Simplify ff prepared rounding check. --------- (cherry picked from commit de59264) * Remove breaking abstract isUTC() getter from Rounding.java. * Remove unused ZoneId getter. --------- (cherry picked from commit a79c6e8) Signed-off-by: Finn Carroll <carrofin@amazon.com> Signed-off-by: bowenlan-amzn <bowenlan23@gmail.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> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
Backport de59264 from #17023.
Additionally update
isUTC
to be non-abstract. Breaks backward compatibility with 2.x.Remove unused
ZoneId
getter from DocValueFormat.java.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.