You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2025/07/15 23:34:49 UTC
[PR] Fix rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Jackie-Jiang opened a new pull request, #16348:
URL: https://github.com/apache/pinot/pull/16348
For pauseless table, COMMITTING segment will be in ONLINE state.
This PR fixes the rebalance logic by treating COMMITTING segments as CONSUMING to avoid relocating COMMITTING segments because that can cause segments to be unavailable.
--
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 rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Posted by "somandal (via GitHub)" <gi...@apache.org>.
somandal commented on PR #16348:
URL: https://github.com/apache/pinot/pull/16348#issuecomment-3080045610
> @KKcorps @somandal @9aman @J-HowHuang Please take a look, and help add a unit test for this fix
@J-HowHuang @noob-se7en can one of you help with the unit tests for this one, I probably won't be able to get to it until after my vacation
--
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 rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16348:
URL: https://github.com/apache/pinot/pull/16348#issuecomment-3076061273
@KKcorps @somandal @9aman @J-HowHuang Please take a look, and help add a unit test for this fix
--
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 rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16348:
URL: https://github.com/apache/pinot/pull/16348#issuecomment-3076705138
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16348?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 56.35%. Comparing base [(`1a476de`)](https://app.codecov.io/gh/apache/pinot/commit/1a476dea72ebf1c15e14ded28ae60c385f71b2c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`910f96e`)](https://app.codecov.io/gh/apache/pinot/commit/910f96e83679e10090ab34a84a701c3e1606ecdc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 452 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (1a476de) and HEAD (910f96e). Click for more details.
>
> <details><summary>HEAD has 68 uploads less than BASE</summary>
>
>| Flag | BASE (1a476de) | HEAD (910f96e) |
>|------|------|------|
>|java-21|10|1|
>|unittests|6|1|
>|unittests1|3|1|
>|skip-bytebuffers-true|4|0|
>|temurin|15|1|
>|unittests2|3|0|
>|skip-bytebuffers-false|8|0|
>|java-11|5|0|
>|integration|9|0|
>|integration2|3|0|
>|integration1|3|0|
>|custom-integration1|3|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16348 +/- ##
============================================
- Coverage 62.90% 56.35% -6.56%
+ Complexity 1386 699 -687
============================================
Files 2867 2320 -547
Lines 163354 128270 -35084
Branches 24952 20437 -4515
============================================
- Hits 102755 72283 -30472
+ Misses 52847 50018 -2829
+ Partials 7752 5969 -1783
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16348/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/16348/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/16348/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/16348/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/16348/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/16348/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/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.35% <ø> (-6.47%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.35% <ø> (-6.56%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.35% <ø> (-6.55%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16348/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.35% <ø> (+0.53%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16348/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/16348?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 rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #16348:
URL: https://github.com/apache/pinot/pull/16348
--
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 rebalance logic to treat COMMITTING segments as CONSUMING [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16348:
URL: https://github.com/apache/pinot/pull/16348#issuecomment-3076186778
> * I think your PR addresses the case where we don't include consuming segments, but if we include consuming segments, they might still be moved, right?
Yes. We still need to rely on peer download when consuming segments are moved, or introduce a new check to wait until commit is done. The change in this PR is to treat COMMITTING segment as CONSUMING instead of ONLINE to prevent periodic task to move them.
> * My understanding is that if a segment is in COMMITTING state, eventually RealtimeSegmentValidationManager will run and fix it up (which means this scenario should be handled eventually to get the segment into a good state). Is this not the case if the segment undergoes rebalance?
Ideally this is the case, but moving COMMITTING segment in periodic task is bad, and can cause consumption to rewind.
--
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