Refactor the filter rewrite optimization

[bowenlan-amzn]

Description

As more code coming into the filter rewrite optimization, it starts to become harder to understand.
Not only making the code review slower and painful, it also will slow down the new contributors into this area. So here comes the refactoring work.

Idea

The refactoring shouldn't change any business logic.
After the refactor, reader can easily find all the important information by just reading the class doc and checking the public methods of all classes.

• Add only declarative code to the Aggregator, while keep the optimization business logic in the new package.
  • Declarative code would be the Context object, which has several public methods to invoke the optimization workflow, combined with a Bridge object to provide the optimization any necessary access to the data in Aggregator.
  • The necessary data can be passed into methods of Bridge. Comparing to saving the field into the Bridge class, this way is more readable because it tells you where this field is actually needed directly from the method name.
  • Other than providing access, Bridge can also host/hide the optimization business logic.

Refactoring

• Split the old huge Helper calss into independent components.
• Tighten up any member access modifier of the components, left the important methods as public.
• Clean the unnecessary references from the components. For example, SearchContext, instead of passing into the OptimizationContext, try to utilize the functions in AggregatorBridge to provide it whenever needed.

Why the name — filter rewrite optimization?

Filter in OpenSearch world has similar meaning as query, while it indicates no relavance scoring calculated.
Rewrite in OpenSearch world can mean transform OpenSearch query into lucene query, or transform a query to perform better.

Generally speaking, the optimization rewrites the aggregation into certain filters to improve performance. Aggregation execution is plain and simple iteration and collection on all matches, while filters can take advantage of the Lucene index to get expected results in log or even constant time.

Benchmark

Using the new tool to trigger benchmark from PR #14464 (comment)

Related Issues

Resolves #14435

Check List

• [ ] Functionality includes testing.
• [ ] API changes companion pull request created, if applicable.
• [ ] Public documentation issue/PR created, if applicable.

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.

[github-actions]

❌ Gradle check result for 1a067ba: 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?

[github-actions]

✅ Gradle check result for 7c491b9: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 85.25799% with 60 lines in your changes missing coverage. Please review.

Project coverage is 71.15%. Comparing base (97c1bf0) to head (86cacab).
Report is 3 commits behind head on main.

Files	Patch %	Lines
...ilterrewrite/FilterRewriteOptimizationContext.java	82.60%	9 Missing and 3 partials ⚠️
...arch/aggregations/bucket/filterrewrite/Helper.java	85.00%	4 Missing and 8 partials ⚠️
...tions/bucket/filterrewrite/PointTreeTraversal.java	88.15%	5 Missing and 4 partials ⚠️
...t/filterrewrite/DateHistogramAggregatorBridge.java	87.03%	1 Missing and 6 partials ⚠️
...ns/bucket/filterrewrite/RangeAggregatorBridge.java	80.00%	2 Missing and 5 partials ⚠️
...arch/aggregations/bucket/filterrewrite/Ranges.java	75.00%	2 Missing and 3 partials ⚠️
...egations/bucket/composite/CompositeAggregator.java	86.95%	2 Missing and 1 partial ⚠️
...ucket/filterrewrite/CompositeAggregatorBridge.java	77.77%	0 Missing and 2 partials ⚠️
.../bucket/histogram/AutoDateHistogramAggregator.java	94.73%	1 Missing ⚠️
...ions/bucket/histogram/DateHistogramAggregator.java	90.90%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #14464      +/-   ##
============================================
- Coverage     71.74%   71.15%   -0.59%     
+ Complexity    62904    62235     -669     
============================================
  Files          5178     5185       +7     
  Lines        295167   295146      -21     
  Branches      42679    42660      -19     
============================================
- Hits         211774   210020    -1754     
- Misses        66011    67800    +1789     
+ Partials      17382    17326      -56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

✅ Gradle check result for 9040f6f: SUCCESS

[github-actions]

✅ Gradle check result for e896927: SUCCESS

[github-actions]

✅ Gradle check result for 8962ee3: SUCCESS

[github-actions]

❕ Gradle check result for 86cacab: UNSTABLE

• TEST FAILURES:

      2 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.classMethod
      1 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.testResizeQueueDown

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[mch2]

Thanks for these changes @bowenlan-amzn I think this is much easier to follow than the original helper class. I think we can keep going with some cleanup but my major concern re concurrent search appears resolved.

[sandeshkr419]

@mch2 @bowenlan-amzn We shouldn't skip changelog for these changes.