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 File Formats #14809

Merged

Conversation

sarthakaggarwal97
Copy link
Contributor

@sarthakaggarwal97 sarthakaggarwal97 commented Jul 18, 2024

Description

Introducing the file formats for composite index. There could be multiple implementations of composite index, star-tree being one of them.

With this change, we introduce four new files.

  1. cid - Composite Index Data
  2. cim - Composite Index Meta
  3. cidvd - Composite Doc Values Data
  4. cidvm - Composite Doc Values Meta

Related Issues

Resolves #14837

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.

@sarthakaggarwal97 sarthakaggarwal97 changed the title Startree fileformat Startree File Formats Jul 18, 2024
Copy link
Contributor

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

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the startree-fileformat branch 3 times, most recently from 94d1928 to 1b5009e Compare July 18, 2024 06:07
Copy link
Contributor

❌ Gradle check result for 94d1928: 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 26337a8: SUCCESS

Copy link
Contributor

✅ Gradle check result for 1b5009e: SUCCESS

Copy link
Contributor

❌ Gradle check result for 1fc9add:

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?

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the startree-fileformat branch 2 times, most recently from 72ef06f to 9250579 Compare July 18, 2024 08:20
Copy link
Contributor

✅ Gradle check result for 72ef06f: SUCCESS

Copy link
Contributor

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

@mgodwan
Copy link
Member

mgodwan commented Jul 18, 2024

Can this PR be broken down?
One way I see is to use separate PRs for format write, format read, and integration with build flow. Otherwise it may take time to review this in a single go.

Copy link
Contributor

✅ Gradle check result for 7ded5ca: SUCCESS

Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 89.72746% with 49 lines in your changes missing coverage. Please review.

Project coverage is 72.07%. Comparing base (758c2aa) to head (e13e400).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...posite/composite99/Composite99DocValuesReader.java 77.10% 11 Missing and 8 partials ⚠️
...eindex/datacube/startree/index/StarTreeValues.java 81.53% 7 Missing and 5 partials ⚠️
...datacube/startree/builder/BaseStarTreeBuilder.java 96.12% 3 Missing and 3 partials ⚠️
...index/datacube/startree/node/InMemoryTreeNode.java 88.46% 0 Missing and 6 partials ⚠️
...posite/composite99/Composite99DocValuesWriter.java 93.47% 1 Missing and 2 partials ⚠️
...tacube/startree/builder/OnHeapStarTreeBuilder.java 92.30% 1 Missing ⚠️
.../startree/fileformats/data/StarTreeDataWriter.java 93.33% 0 Missing and 1 partial ⚠️
...rtree/fileformats/meta/StarTreeMetadataWriter.java 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #14809      +/-   ##
============================================
+ Coverage     72.02%   72.07%   +0.04%     
- Complexity    63769    63904     +135     
============================================
  Files          5249     5249              
  Lines        297795   298087     +292     
  Branches      43034    43071      +37     
============================================
+ Hits         214480   214838     +358     
+ Misses        65735    65633     -102     
- Partials      17580    17616      +36     

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

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the startree-fileformat branch 2 times, most recently from 8946d85 to 40015c4 Compare July 19, 2024 10:02
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review July 19, 2024 10:06
@bharath-techie
Copy link
Contributor

Thanks for the changes @sarthakaggarwal97 , this is a really important and tricky PR , thanks for addressing all the comments, adding extensive tests and refactoring the code.

LGTM.

Copy link
Member

@mgodwan mgodwan left a comment

Choose a reason for hiding this comment

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

Overall changes LGTM. Thanks for working through the changes.
Left some minor comments to be addressed

Signed-off-by: Sarthak Aggarwal <[email protected]>
Copy link
Contributor

github-actions bot commented Sep 2, 2024

❕ Gradle check result for e13e400: UNSTABLE

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

@sachinpkale sachinpkale merged commit 6bae704 into opensearch-project:main Sep 2, 2024
34 of 37 checks passed
@sarthakaggarwal97 sarthakaggarwal97 added the backport 2.x Backport to 2.x branch label Sep 2, 2024
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 2, 2024
---------

Signed-off-by: Sarthak Aggarwal <[email protected]>
(cherry picked from commit 6bae704)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gbbafna pushed a commit that referenced this pull request Sep 2, 2024
---------


(cherry picked from commit 6bae704)

Signed-off-by: Sarthak Aggarwal <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
akolarkunnu pushed a commit to akolarkunnu/OpenSearch that referenced this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request Indexing:Performance Indexing & Search Indexing Indexing, Bulk Indexing and anything related to indexing Search:Aggregations skip-changelog v2.16.0 Issues and PRs related to version 2.16.0 v2.17.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Star Tree File Formats
5 participants