You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "leguo2 (via GitHub)" <gi...@apache.org> on 2026/03/24 06:23:11 UTC
[PR] Fix incorrect stream partition id for multi-stream realtime consumption [pinot]
leguo2 opened a new pull request, #17953:
URL: https://github.com/apache/pinot/pull/17953
### Summary
This PR fixes a bug in multi-stream realtime consumption where the server can use the Pinot `partitionGroupId`
instead of the actual upstream `streamPartitionId` when creating the consuming state for a segment.
The server already derives the correct `_streamPartitionId`, but when constructing
`PartitionGroupConsumptionStatus` in `RealtimeSegmentDataManager`, it previously used the constructor that defaults:
`streamPartitionId = partitionGroupId`
As a result, the Kafka consumer could incorrectly consume partition `10004` instead of partition `4`.
This change updates `RealtimeSegmentDataManager` to explicitly pass the derived `_streamPartitionId`
when creating `PartitionGroupConsumptionStatus`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix incorrect stream partition id for multi-stream realtime consumption [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17953:
URL: https://github.com/apache/pinot/pull/17953#issuecomment-4116024121
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17953?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 55.48%. Comparing base ([`be3ad3b`](https://app.codecov.io/gh/apache/pinot/commit/be3ad3bc17db909f05c418a3e0b97e60c4fd01f4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f65af22`](https://app.codecov.io/gh/apache/pinot/commit/f65af22343d84927fd7da92adc9d943669aef579?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (be3ad3b) and HEAD (f65af22). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (be3ad3b) | HEAD (f65af22) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration1|2|0|
>|integration2|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17953 +/- ##
============================================
- Coverage 63.21% 55.48% -7.74%
+ Complexity 1525 729 -796
============================================
Files 3194 2497 -697
Lines 193239 142254 -50985
Branches 29706 22786 -6920
============================================
- Hits 122161 78925 -43236
+ Misses 61494 56609 -4885
+ Partials 9584 6720 -2864
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <100.00%> (-7.72%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <100.00%> (-7.74%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <100.00%> (-7.74%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <100.00%> (-0.05%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17953/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/17953?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix incorrect stream partition id for multi-stream realtime consumption [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #17953:
URL: https://github.com/apache/pinot/pull/17953
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix incorrect stream partition id for multi-stream realtime consumption [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17953:
URL: https://github.com/apache/pinot/pull/17953#discussion_r2979935605
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -1756,7 +1756,7 @@ public RealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableConf
String streamTopic = _streamConfig.getTopicName();
_segmentNameStr = _segmentZKMetadata.getSegmentName();
_partitionGroupConsumptionStatus =
- new PartitionGroupConsumptionStatus(_partitionGroupId, llcSegmentName.getSequenceNumber(),
+ new PartitionGroupConsumptionStatus(_partitionGroupId, _streamPartitionId, llcSegmentName.getSequenceNumber(),
_streamPartitionMsgOffsetFactory.create(_segmentZKMetadata.getStartOffset()),
Review Comment:
This fixes a multi-stream bug, but there’s no unit test exercising the multi-stream branch where `partitionGroupId` encodes the stream index (e.g., 10000) and `_streamPartitionId` is derived via `IngestionConfigUtils.getStreamPartitionIdFromPinotPartitionId`. Consider adding a test that builds a `RealtimeSegmentDataManager` with a multi-stream `IngestionConfig` and asserts the constructed `PartitionGroupConsumptionStatus` carries the derived stream partition id (not the partitionGroupId), so this regression can’t recur.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix incorrect stream partition id for multi-stream realtime consumption [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #17953:
URL: https://github.com/apache/pinot/pull/17953#issuecomment-4115835663
Good catch, thanks for fixing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org