Skip to content
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] Improve string terms aggregation performance using Collector#setWeight #12629

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

sandeshkr419
Copy link
Contributor

@sandeshkr419 sandeshkr419 commented Mar 13, 2024

TLDR: The PR is a combination of 2 PRs merged in main: #11643 & #12643

Description

Test automatically merges the index after delete sometimes leading to flaky test. Avoiding that from happening to stabilize test.
Follow-up on #12628
Backport 7dac98c from #11643.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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.

…onally match-all for a segment (opensearch-project#11643)



---------

Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
Copy link
Contributor

Compatibility status:

Checks if related components are compatible with change bcb6c39

Incompatible components

Incompatible components: [https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/sql.git]

Copy link
Contributor

❕ Gradle check result for bcb6c39: UNSTABLE

  • TEST FAILURES:
      2 org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction
      1 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=indices.get_field_mapping/20_missing_field/Return empty object if field doesn't exist, but index does}

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

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 85.10638% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 71.19%. Comparing base (0dd892c) to head (bcb6c39).
Report is 12 commits behind head on 2.x.

Files Patch % Lines
...ket/terms/GlobalOrdinalsStringTermsAggregator.java 84.44% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #12629      +/-   ##
============================================
- Coverage     71.28%   71.19%   -0.09%     
+ Complexity    60145    60130      -15     
============================================
  Files          4957     4957              
  Lines        282799   282866      +67     
  Branches      41409    41425      +16     
============================================
- Hits         201591   201397     -194     
- Misses        64189    64446     +257     
- Partials      17019    17023       +4     

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

@sandeshkr419
Copy link
Contributor Author

sandeshkr419 commented Mar 13, 2024

@msfroh Closed auto-backport PR: #12628 since 2.x was somehow merging up test indices. Modified test-index a bit to not merge at all - see the additional commit.

Also, please review this.

Edit: Included the extra commit in main as well. PR: #12643

@sandeshkr419 sandeshkr419 self-assigned this Mar 13, 2024
@sandeshkr419 sandeshkr419 added the backport PRs or issues specific to backporting features or enhancments label Mar 13, 2024
@sandeshkr419
Copy link
Contributor Author

sandeshkr419 commented Mar 13, 2024

@reta Can you please check (& possibly merge) this - I see you already have context here.

The PR is a combination/backport of 2 PRs merged in main: #11643 & #12643

@reta
Copy link
Collaborator

reta commented Mar 13, 2024

@reta Can you please check (& possibly merge) this - I see you already have context here.

I haven't checked the original pull request closely, I would leave it to @msfroh

@msfroh msfroh merged commit b1c8629 into opensearch-project:2.x Mar 13, 2024
100 of 101 checks passed
@sandeshkr419 sandeshkr419 deleted the agg-per branch March 13, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport PRs or issues specific to backporting features or enhancments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants