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 SegmentContext to collect validDocIds bitmaps for many segments together #12694

Merged
merged 11 commits into from
Apr 9, 2024

Conversation

klsince
Copy link
Contributor

@klsince klsince commented Mar 21, 2024

This PR adds SegmentContext used to collect segment related context before query execution. E.g. this makes it easier to collect validDocIds for many segments together, so that we can add the support for consistent table view for upsert table easier later on.

This PR didn't change current functionalities but moved the logic of getting validDocId bitmaps upto one place, i.e. tableDataMgr.getSegmentContexts(selectedSegments).

@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2024

Codecov Report

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

Project coverage is 62.04%. Comparing base (59551e4) to head (a13897e).
Report is 244 commits behind head on master.

Files Patch % Lines
...cal/upsert/BasePartitionUpsertMetadataManager.java 56.25% 14 Missing ⚠️
...ata/manager/realtime/RealtimeTableDataManager.java 0.00% 5 Missing ⚠️
...psert/ConcurrentMapTableUpsertMetadataManager.java 0.00% 3 Missing ⚠️
...core/query/executor/ServerQueryExecutorV1Impl.java 92.85% 1 Missing ⚠️
...t/ConcurrentMapPartitionUpsertMetadataManager.java 50.00% 0 Missing and 1 partial ⚠️
...gment/local/upsert/TableUpsertMetadataManager.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #12694      +/-   ##
============================================
+ Coverage     61.75%   62.04%   +0.28%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2464      +28     
  Lines        133233   134762    +1529     
  Branches      20636    20813     +177     
============================================
+ Hits          82274    83607    +1333     
- Misses        44911    45002      +91     
- Partials       6048     6153     +105     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 34.95% <70.06%> (-26.76%) ⬇️
java-21 61.92% <82.99%> (+0.30%) ⬆️
skip-bytebuffers-false 62.00% <82.99%> (+0.25%) ⬆️
skip-bytebuffers-true 61.86% <82.99%> (+34.14%) ⬆️
temurin 62.04% <82.99%> (+0.28%) ⬆️
unittests 62.03% <82.99%> (+0.28%) ⬆️
unittests1 46.69% <70.06%> (-0.20%) ⬇️
unittests2 27.99% <12.92%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@tibrewalpratik17 tibrewalpratik17 left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for refactoring!

@klsince klsince requested a review from Jackie-Jiang March 25, 2024 18:10
@klsince klsince force-pushed the validdocids_as_segment_context branch from 2f92034 to 080a453 Compare April 3, 2024 21:16
@klsince klsince force-pushed the validdocids_as_segment_context branch from 6926891 to a353b11 Compare April 5, 2024 23:33
@Jackie-Jiang Jackie-Jiang added the incompatible Indicate PR that introduces backward incompatibility label Apr 8, 2024
@klsince klsince merged commit 101af7c into apache:master Apr 9, 2024
19 checks passed
@klsince klsince deleted the validdocids_as_segment_context branch April 9, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup enhancement incompatible Indicate PR that introduces backward incompatibility upsert
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants