Skip to content

Add validations for Pauseless Tables #15567

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.

Sign up for GitHub

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

Jump to bottom
Merged
KKcorps merged 3 commits into from
Apr 25, 2025
Merged

Add validations for Pauseless Tables #15567

KKcorps merged 3 commits into from
Apr 25, 2025

Conversation

@KKcorps
Copy link
Contributor

@KKcorps KKcorps commented Apr 16, 2025

  • Enforce the peerSegmentDownloadScheme is always set

noob-se7en
noob-se7en approved these changes Apr 16, 2025
View reviewed changes
Copy link
Contributor

@noob-se7en noob-se7en left a comment

Choose a reason for hiding this comment

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

Not a table config but a controller flag:
Seems like this also needs to be enabled for pausless tables: controller.segment.error.autoReset

@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.

Project coverage is 62.81%. Comparing base (97aa5ca) to head (b2d01d5).
Report is 35 commits behind head on master.

Files with missing lines Patch % Lines
...he/pinot/segment/local/utils/TableConfigUtils.java 9.09% 7 Missing and 3 partials ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #15567      +/-   ##
============================================
+ Coverage     62.79%   62.81%   +0.01%     
  Complexity     1384     1384              
============================================
  Files          2864     2865       +1     
  Lines        162665   162887     +222     
  Branches      24901    24928      +27     
============================================
+ Hits         102143   102311     +168     
- Misses        52822    52867      +45     
- Partials       7700     7709       +9
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 62.75% <9.09%> (-0.02%) ⬇️
java-21 62.78% <9.09%> (+<0.01%) ⬆️
skip-bytebuffers-false 62.80% <9.09%> (+0.01%) ⬆️
skip-bytebuffers-true 62.73% <9.09%> (-0.05%) ⬇️
temurin 62.81% <9.09%> (+0.01%) ⬆️
unittests 62.80% <9.09%> (+0.01%) ⬆️
unittests1 55.73% <0.00%> (-0.02%) ⬇️
unittests2 33.57% <9.09%> (+0.04%) ⬆️

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.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KKcorps KKcorps force-pushed the pauseless_table_validation branch from 30be0ed to 7d9b364 Compare April 17, 2025 16:34
@KKcorps KKcorps force-pushed the pauseless_table_validation branch from 7d9b364 to e677c85 Compare April 18, 2025 11:17
@KKcorps KKcorps force-pushed the pauseless_table_validation branch from f464144 to b2d01d5 Compare April 25, 2025 12:01
xiangfu0
xiangfu0 approved these changes Apr 25, 2025
View reviewed changes
Copy link
Contributor

@xiangfu0 xiangfu0 left a comment

Choose a reason for hiding this comment

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

Not related to this issue, for peer download, is there a way to just tarball the data but not gzip?
Considering the network environment is usually good for p2p download. The e2e time will be spent on compression but not data transfer.

@KKcorps KKcorps merged commit 0fc9ec6 into apache:master Apr 25, 2025
22 checks passed
@KKcorps KKcorps mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jackie-Jiang Jackie-Jiang Jackie-Jiang left review comments

@xiangfu0 xiangfu0 xiangfu0 approved these changes

+1 more reviewer

@noob-se7en noob-se7en noob-se7en approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ingestion real-time

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@KKcorps @codecov-commenter @xiangfu0 @Jackie-Jiang @noob-se7en