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

[Star Tree] Multiple Star Trees and Added Tests #16124

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sarthakaggarwal97
Copy link
Contributor

Description

This changes allows us to make multiple star trees (2 for now) per segment. This test also includes the tests to validate multiple star trees.

Related Issues

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

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.

Copy link
Contributor

❌ Gradle check result for 024e21a: 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: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Copy link
Contributor

github-actions bot commented Oct 7, 2024

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

@@ -35,7 +35,7 @@ public class StarTreeIndexSettings {
"index.composite_index.star_tree.max_fields",
1,
1,
1,
2,
Copy link
Contributor

Choose a reason for hiding this comment

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

By doing this, aren't we allowing users as well to create multiple star trees ? can we check if there are any better ways to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we can wait to merge this PR. Since you have taken the fixes in the #16380, logically we should not face issues in multi star tree.
Once we would want to support the multiple star tree, we can merge this PR alongside the test.

@sarthakaggarwal97
Copy link
Contributor Author

sarthakaggarwal97 commented Oct 18, 2024

Multi Star Tree fixes are now taken over here: #16380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants