Skip to content

[bugfix] Fix per-stream partition count in segment metadata for consuming segments in multi-stream tables#18401

Merged
xiangfu0 merged 2 commits into
apache:masterfrom
shauryachats:multitopic_consumingpartitionfix
May 6, 2026
Merged

[bugfix] Fix per-stream partition count in segment metadata for consuming segments in multi-stream tables#18401
xiangfu0 merged 2 commits into
apache:masterfrom
shauryachats:multitopic_consumingpartitionfix

Conversation

@shauryachats
Copy link
Copy Markdown
Collaborator

Problem

For multi-stream realtime tables, getPartitionMetadataFromTableConfig was storing numPartitionGroups (total across all streams) in ColumnPartitionMetadata.numPartitions.

This is incorrect - the broker's partition pruning compares that value against the per-stream partition count from the partition function. Using the total inflated the count by a factor of numStreams, causing pruning to silently skip segments it should have matched.

Fix

  • Compute perStreamNumPartitions = numPartitionGroups / numStreams and use it in ColumnPartitionMetadata, consistent with what the broker's partition function expects.
  • Return null early (skip persisting partition metadata) when numPartitionGroups is not evenly divisible by numStreams, logging a warning. This avoids storing metadata that would produce incorrect pruning results (and null means segment will always be included).
  • Single-stream tables are unaffected (perStreamNumPartitions = numPartitionGroups).

Tests

Added testGetPartitionMetadataFromTableConfig covering:

  • No SegmentPartitionConfignull
  • Single-stream: partition count equals total partition groups (no change)
  • Multi-stream, even distribution: partition count equals numPartitionGroups / numStreams
  • Multi-stream, uneven distribution: returns null

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 2, 2026

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.47%. Comparing base (b5dca33) to head (710284e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../core/realtime/PinotLLCRealtimeSegmentManager.java 70.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #18401   +/-   ##
=========================================
  Coverage     63.46%   63.47%           
  Complexity     1701     1701           
=========================================
  Files          3254     3254           
  Lines        199104   199109    +5     
  Branches      30830    30832    +2     
=========================================
+ Hits         126365   126380   +15     
+ Misses        62655    62645   -10     
  Partials      10084    10084           
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (?)
java-21 63.47% <70.00%> (+<0.01%) ⬆️
temurin 63.47% <70.00%> (+<0.01%) ⬆️
unittests 63.46% <70.00%> (+<0.01%) ⬆️
unittests1 55.41% <ø> (-0.01%) ⬇️
unittests2 35.00% <70.00%> (+<0.01%) ⬆️

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.

Copy link
Copy Markdown
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.

Found one high-signal issue; see inline comment.

@xiangfu0 xiangfu0 added ingestion Related to data ingestion pipeline bug Something is not working as expected labels May 2, 2026
@xiangfu0
Copy link
Copy Markdown
Contributor

xiangfu0 commented May 2, 2026

I feel this is backward incompatible behavior change instead of a bugfix.

@xiangfu0 xiangfu0 added backward-incompat Introduces a backward-incompatible API or behavior change and removed bug Something is not working as expected labels May 2, 2026
@xiangfu0
Copy link
Copy Markdown
Contributor

xiangfu0 commented May 2, 2026

Consider either adding a new config on how to compute the total partitions, or just configuring the total number of partitions for the table.

@shauryachats
Copy link
Copy Markdown
Collaborator Author

shauryachats commented May 2, 2026

@xiangfu0 this is not a backward-incompatible change since this bug only fires for segments which are created in consuming state.

At commit time the server writes the segment ZK metadata via SegmentZKMetadataUtils.updateCommittingSegmentZKMetadata, which reads numPartitions directly from the physical segment file — a value embedded by StatelessRealtimeSegmentWriter.setPartitionParameters on the server, whose _partitionMetadataProvider is scoped to a single stream's config and therefore always returns the correct per-stream count.

Hence, even when this fix is rolling out, the segments with the incorrect partitions value anyways get fixed when they transition from CONSUMING -> ONLINE. That is also the reason when we enabled broker pruning for multi-topic ingested tables, we were seeing this issue only for data in the consuming segments (the committed segments already had the correct per-stream partition count), and once this fix rolled out internally, we confirmed that both the consuming segments and all the committed segments had the correct per-stream partition count for each segment.

@shauryachats
Copy link
Copy Markdown
Collaborator Author

@xiangfu0 @ankitsultana could you please help review? I have double verified this on a production cluster internally that validates this fix is not backward incompatible.

@xiangfu0 xiangfu0 merged commit cedb6c6 into apache:master May 6, 2026
11 checks passed
@shauryachats shauryachats added bug Something is not working as expected and removed backward-incompat Introduces a backward-incompatible API or behavior change labels May 6, 2026
@shauryachats shauryachats deleted the multitopic_consumingpartitionfix branch May 6, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is not working as expected ingestion Related to data ingestion pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants