Apply the fast filter optimization to composite aggregation

[bowenlan-amzn]

Description

This PR adds the fast filter optimization to composite aggregation (comp-agg).
The optimization only applies when date histogram is the only source of comp-agg and no parent or sub aggregations.
The optimization only applies when the query is a range query on the same date field or is a MatchAllDocsQuery
The optimization acts on segment level, the rewritted range filters take in leaf context and give the counts of each range.

Refactor

• Move the tryFastFilterAggregation from collect to getLeafCollector. Because if this try succeed, we don't need to return a collector to collect later. The results (doc counts of buckets) are saved at Aggregator or Shard level and will be processed by buildAggregations method later.

Glossary

• slot: the bucket ordinal in comp-agg queue which refers to the aggregation result (key and doc count)

Related Issues

Resolves #11301

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)
• Public documentation issue/PR created

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]

Compatibility status:

Checks if related components are compatible with change ce81e3d

Incompatible components

Incompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git]

[github-actions]

❌ Gradle check result for 36f98e7: null

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 06a007e: 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?

[bowenlan-amzn]

PR Analysis

• 🎯 Main theme: Optimization of composite aggregation in OpenSearch
• 📝 PR summary: This PR introduces an optimization to the composite aggregation in OpenSearch. It applies a fast filter optimization to the composite aggregation, which can significantly improve the performance of certain queries. The PR also includes changes to several related classes and adds new tests to verify the functionality.
• 📌 Type of PR: Enhancement
• 🧪 Relevant tests added: Yes
• ⏱️ Estimated effort to review [1-5]: 4, because the PR includes a significant amount of new logic and changes to existing classes. It requires a good understanding of the OpenSearch aggregation framework to review effectively.
• 🔒 Security concerns: No security concerns found

PR Feedback

• 💡 General suggestions: The PR is generally well-structured and follows good coding practices. However, it would be beneficial to include more detailed comments explaining the logic behind the new optimization, especially in complex methods. This will make the code easier to understand for other developers. Also, consider adding more context to the PR description to help reviewers understand the changes better.

• 🤖 Code feedback:

  • relevant file: server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java
    suggestion: Consider refactoring the CompositeAggregator constructor to reduce its complexity and improve readability. The constructor is currently quite long and does a lot of different things, which can make it hard to understand. You could potentially extract some of the logic into separate methods. [important]
    relevant line: "+ CompositeAggregator("

  • relevant file: server/src/main/java/org/opensearch/search/aggregations/bucket/FilterRewriteHelper.java
    suggestion: It would be beneficial to add more detailed comments explaining the purpose and functionality of the FilterRewriteHelper class and its methods. This will make the code easier to understand for other developers. [medium]
    relevant line: "+public class FilterRewriteHelper {"

  • relevant file: server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java
    suggestion: Consider adding null checks or assertions for the 'filters', 'bucketOrds', 'fieldType', and 'preparedRounding' fields. This can help prevent potential NullPointerExceptions. [important]
    relevant line: "+ private Weight[] filters = null;"

  • relevant file: server/src/main/java/org/opensearch/search/aggregations/bucket/composite/CompositeAggregator.java
    suggestion: Consider refactoring the 'buildAggregations' method to reduce its complexity and improve readability. The method is currently quite long and does a lot of different things, which can make it hard to understand. You could potentially extract some of the logic into separate methods. [important]
    relevant line: "+ public InternalAggregation[] buildAggregations(long[] owningBucketOrds) throws IOException {"

[github-actions]

❌ Gradle check result for d8f337b: 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 e7d8af9: null

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 589d03c: 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 3a621c4: 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 c6874c1: 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 f2a220f: 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?

[bowenlan-amzn]

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

Confirm all the failures are already listed in flaky test issues
#10558
#9191
#10758
#8945
#11091

[github-actions]

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

[bowenlan-amzn]

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

Confirmed it's flaky test #9605

[github-actions]

❕ Gradle check result for ce81e3d: UNSTABLE

• TEST FAILURES:

      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

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