You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "lnbest0707-uber (via GitHub)" <gi...@apache.org> on 2025/08/26 07:39:13 UTC
[PR] [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [pinot]
lnbest0707-uber opened a new pull request, #16692:
URL: https://github.com/apache/pinot/pull/16692
`real-time` `ingestion` `feature`
Part 2 of https://github.com/apache/pinot/pull/15782, but this is independent from part 1
Issue https://github.com/apache/pinot/issues/14815
Design doc https://docs.google.com/document/d/1NKPeNh6V2ctaQ4T_X3OKJ6Gcy5TRanLiU1uIDT8_9UA/edit?usp=sharing
This is an alternative PR of https://github.com/apache/pinot/pull/16494
Instead of introducing the new segment naming format that concerns some parties, this PR chooses the way to "soft delete" the topics by marking it as 'inactive'.
We still cannot literally delete a topic from the table config.
But instead, we would introduce a similar "pause" consumption API to mark topic as 'inactive' in the idealstae. As a result, the consumption of the topic would stop.
This 'inactive' marking would also prevent the metadata being queries from the stream. So that it would be safe to delete the topic from the stream.
--
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] [Auto reset 2/3]Introduce topic 'inactive' status [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat merged PR #16692:
URL: https://github.com/apache/pinot/pull/16692
--
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] [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on code in PR #16692:
URL: https://github.com/apache/pinot/pull/16692#discussion_r2305568457
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/PauseState.java:
##########
@@ -26,15 +28,19 @@ public class PauseState extends BaseJsonConfig {
private ReasonCode _reasonCode;
private String _comment;
private String _timestamp;
+ // List of inactive topic indices. Index is the index of the topic in the streamConfigMaps.
+ private List<Integer> _indexOfInActiveTopics;
Review Comment:
InActiveTopics ==> InactiveTopics
--
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] [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [pinot]
Posted by "lnbest0707-uber (via GitHub)" <gi...@apache.org>.
lnbest0707-uber commented on code in PR #16692:
URL: https://github.com/apache/pinot/pull/16692#discussion_r2301782907
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -479,7 +479,7 @@ private void updateSegmentMetrics(String tableNameWithType, TableConfig tableCon
if (tableType == TableType.REALTIME && tableConfig != null) {
List<StreamConfig> streamConfigs = IngestionConfigUtils.getStreamConfigs(tableConfig);
new MissingConsumingSegmentFinder(tableNameWithType, propertyStore, _controllerMetrics,
- streamConfigs).findAndEmitMetrics(idealState);
+ streamConfigs, idealState).findAndEmitMetrics(idealState);
Review Comment:
Added to the description
--
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] [Auto reset 2/3]Introduce topic 'inactive' status [pinot]
Posted by "noob-se7en (via GitHub)" <gi...@apache.org>.
noob-se7en commented on PR #16692:
URL: https://github.com/apache/pinot/pull/16692#issuecomment-3298796891
Please fix this:
```
"org.apache.pinot.spi.config.table.PauseState.getIndexOfInactiveTopics()" is null
at org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager.isTopicPaused(PinotLLCRealtimeSegmentManager.java:1409)
at org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager.isTopicPaused(PinotLLCRealtimeSegmentManager.java:1417)
at org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager.createNewSegmentMetadata(PinotLLCRealtimeSegmentManager.java:785)
at org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager.commitSegmentMetadataInternal(PinotLLCRealtimeSegmentManager.java:659)
at org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager.commitSegmentMetadata(PinotLLCRealtimeSegmentManager.java:623)
at org.apache.pinot.controller.helix.core.realtime.BlockingSegmentCompletionFSM.commitSegmentMetadata(BlockingSegmentCompletionFSM.java:844)
at org.apache.pinot.controller.helix.core.realtime.BlockingSegmentCompletionFSM.commitSegment(BlockingSegmentCompletionFSM.java:829)
at org.apache.pinot.controller.helix.core.realtime.BlockingSegmentCompletionFSM.segmentCommitEnd(BlockingSegmentCompletionFSM.java:388)
at org.apache.pinot.controller.helix.core.realtime.SegmentCompletionManager.segmentCommitEnd(SegmentCompletionManager.java:322)
at org.apache.pinot.controller.api.resources.LLCSegmentCompletionHandlers.segmentCommitEndWithMetadata(LLCSegmentCompletionHandlers.java:303)
```
--
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] [Auto reset 2/3]Introduce topic 'inactive' status [pinot]
Posted by "lnbest0707-uber (via GitHub)" <gi...@apache.org>.
lnbest0707-uber commented on PR #16692:
URL: https://github.com/apache/pinot/pull/16692#issuecomment-3235282300
> @lnbest0707-uber can you check if the test failures are related?
[Pinot Binary Compatibility Check (pull_request)](https://github.com/apache/pinot/actions/runs/17281511432/job/49050635747?pr=16692) is due to changing the constructor. But since we will use the new and discontinue using the old. I feel I should remove the old instead keeping both.
The integration test succeeds on Java21 and should be flaky.
--
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] [Auto reset 2/3]Introduce topic 'inactive' status [pinot]
Posted by "chenboat (via GitHub)" <gi...@apache.org>.
chenboat commented on PR #16692:
URL: https://github.com/apache/pinot/pull/16692#issuecomment-3235259658
@lnbest0707-uber can you check if the test failures are related?
--
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] [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16692:
URL: https://github.com/apache/pinot/pull/16692#issuecomment-3224415756
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16692?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `22.61905%` with `65 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 63.30%. Comparing base ([`777936a`](https://app.codecov.io/gh/apache/pinot/commit/777936a28e50fc075b934b4c038b29a2e6832e86?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`73bc55e`](https://app.codecov.io/gh/apache/pinot/commit/73bc55e5295189a94ce38af8eeddd18856279e3e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 7 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/16692?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fhelix%2Fcore%2Frealtime%2FPinotLLCRealtimeSegmentManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | 19.11% | [51 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../org/apache/pinot/spi/config/table/PauseState.java](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fconfig%2Ftable%2FPauseState.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1BhdXNlU3RhdGUuamF2YQ==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/spi/stream/PartitionGroupMetadataFetcher.java](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fstream%2FPartitionGroupMetadataFetcher.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1BhcnRpdGlvbkdyb3VwTWV0YWRhdGFGZXRjaGVyLmphdmE=) | 33.33% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...x/core/realtime/MissingConsumingSegmentFinder.java](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fhelix%2Fcore%2Frealtime%2FMissingConsumingSegmentFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL01pc3NpbmdDb25zdW1pbmdTZWdtZW50RmluZGVyLmphdmE=) | 75.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16692 +/- ##
============================================
- Coverage 63.33% 63.30% -0.04%
Complexity 1379 1379
============================================
Files 3036 3036
Lines 176785 176899 +114
Branches 27131 27130 -1
============================================
+ Hits 111971 111987 +16
- Misses 56227 56310 +83
- Partials 8587 8602 +15
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16692/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/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.26% <22.61%> (-0.04%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.27% <22.61%> (-0.04%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.30% <22.61%> (-0.04%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.30% <22.61%> (-0.04%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.45% <0.00%> (+0.03%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16692/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.12% <22.61%> (-0.04%)` | :arrow_down: |
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/16692?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] [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [pinot]
Posted by "deemoliu (via GitHub)" <gi...@apache.org>.
deemoliu commented on code in PR #16692:
URL: https://github.com/apache/pinot/pull/16692#discussion_r2301730868
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -479,7 +479,7 @@ private void updateSegmentMetrics(String tableNameWithType, TableConfig tableCon
if (tableType == TableType.REALTIME && tableConfig != null) {
List<StreamConfig> streamConfigs = IngestionConfigUtils.getStreamConfigs(tableConfig);
new MissingConsumingSegmentFinder(tableNameWithType, propertyStore, _controllerMetrics,
- streamConfigs).findAndEmitMetrics(idealState);
+ streamConfigs, idealState).findAndEmitMetrics(idealState);
Review Comment:
is this a active/inactive flag for table level or topic level?
can you please add an example on the ideal state changes?
--
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