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

Add cluster setting to dynamically disable filter rewrite optimization #13179

Merged

Conversation

bowenlan-amzn
Copy link
Member

@bowenlan-amzn bowenlan-amzn commented Apr 13, 2024

Short term solution for pmc workload regression #13171

Description

Reduce the deciding threshold for rewrite filters from 1024 to 24. Meaning if the date histogram aggregation include more than 24 buckets (e.g. hourly aggregation of 1 day), we won't use the optimization

After this change, we will probably see regression for date_histogram_hourly_agg of big5 workload. That will be handled after the long term solution merged in.

Related Issues

#13171

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.

Short term solution for pmc workload regression opensearch-project#13171

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
Copy link
Contributor

github-actions bot commented Apr 13, 2024

Compatibility status:

Checks if related components are compatible with change 4bded8f

Incompatible components

Skipped components

Compatible components

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

Copy link
Contributor

✅ Gradle check result for 386a24b: SUCCESS

Copy link

codecov bot commented Apr 14, 2024

Codecov Report

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

Project coverage is 71.37%. Comparing base (b15cb0c) to head (4bded8f).
Report is 169 commits behind head on main.

Files Patch % Lines
...h/aggregations/bucket/FastFilterRewriteHelper.java 0.00% 3 Missing and 1 partial ⚠️
...va/org/opensearch/search/DefaultSearchContext.java 80.00% 1 Missing ⚠️
.../org/opensearch/search/internal/SearchContext.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13179      +/-   ##
============================================
- Coverage     71.42%   71.37%   -0.05%     
- Complexity    59978    60598     +620     
============================================
  Files          4985     5040      +55     
  Lines        282275   285381    +3106     
  Branches      40946    41329     +383     
============================================
+ Hits         201603   203698    +2095     
- Misses        63999    64807     +808     
- Partials      16673    16876     +203     

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

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jainankitk jainankitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Also, need to create documentation issue and PR for this change.

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
@bowenlan-amzn
Copy link
Member Author

bowenlan-amzn commented Apr 15, 2024

https://build.ci.opensearch.org/job/gradle-check/37020/
https://build.ci.opensearch.org/job/gradle-check/37020/testReport/junit/org.opensearch.repositories.gcs/GoogleCloudStorageBlobStoreRepositoryTests/

org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testDeleteSingleItem
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testDeleteBlobs
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testWriteReadLarge
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testRequestStats
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testIndicesDeletedFromRepository
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testMultipleSnapshotAndRollback
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testWriteRead
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testSnapshotWithLargeSegmentFiles
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testList
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testContainerCreationAndDeletion
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testReadRange
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testSnapshotAndRestore
org.opensearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests.testReadNonExistingPath

The error is like this

StorageException[Error getting access token for service account: Read timed out, iss: opensearch@appspot.gserviceaccount.com]

#8439 seems report similar error

@bowenlan-amzn
Copy link
Member Author

LGTM! Also, need to create documentation issue and PR for this change.

Created a documentation PR:
opensearch-project/documentation-website#6957

Copy link
Contributor

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

@bowenlan-amzn bowenlan-amzn added the backport 2.x Backport to 2.x branch label Apr 15, 2024
Copy link
Contributor

❌ Gradle check result for 0667396: 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: bowenlan-amzn <bowenlan23@gmail.com>
@bowenlan-amzn
Copy link
Member Author

@github-actions commented on Apr 15, 2024, 3:14 PM PDT:

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

Originally posted by @github-actions[bot] in #13179 (comment)

org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction
org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction
org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction
org.opensearch.cluster.coordination.AwarenessAttributeDecommissionIT.testConcurrentDecommissionAction

Flaky tests tracked in #12197

Copy link
Contributor

❌ Gradle check result for 4bded8f:

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?

Copy link
Contributor

✅ Gradle check result for 4bded8f: SUCCESS

@msfroh msfroh merged commit 035d8b8 into opensearch-project:main Apr 16, 2024
58 of 62 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-13179-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 035d8b8e8bcbeefea73b67f6ab753d5b075227a7
# Push it to GitHub
git push --set-upstream origin backport/backport-13179-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-13179-to-2.x.

@bowenlan-amzn bowenlan-amzn deleted the hardening_for_pmc_regression branch April 16, 2024 01:14
bowenlan-amzn added a commit to bowenlan-amzn/OpenSearch that referenced this pull request Apr 16, 2024
@bowenlan-amzn
Copy link
Member Author

Manually backport #13217

msfroh pushed a commit that referenced this pull request Apr 16, 2024
…ion (#13179) (#13217)

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
bowenlan-amzn added a commit to bowenlan-amzn/OpenSearch that referenced this pull request Apr 19, 2024
…ion (opensearch-project#13179)

---------

Signed-off-by: bowenlan-amzn <bowenlan23@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants