mcvsubbu commented on code in PR #13584:
URL: https://github.com/apache/pinot/pull/13584#discussion_r1763445783
##########
pinot-controller/src/main/java/org/apache/pinot/controller/validation/RealtimeSegmentValidationManager.java:
##########
@@ -108,8 +112,45 @@ protected void processTable(String tableNameWithType,
Context context) {
if (context._runSegmentLevelValidation) {
runSegmentLevelValidation(tableConfig, streamConfig);
}
- _llcRealtimeSegmentManager.ensureAllPartitionsConsuming(tableConfig,
streamConfig,
- context._recreateDeletedConsumingSegment, context._offsetCriteria);
+
+ if (shouldEnsureConsuming(tableNameWithType, context)) {
+ _llcRealtimeSegmentManager.ensureAllPartitionsConsuming(tableConfig,
streamConfig,
+ context._recreateDeletedConsumingSegment, context._offsetCriteria);
+ }
+ }
+
+ private boolean shouldEnsureConsuming(String tableNameWithType, Context
context) {
+ // Keeps the table paused/unpaused based pause validations.
+ // Skips updating the pause state if table is paused by admin
+ PauseState pauseState = computePauseState(tableNameWithType);
+ if (!pauseState.isPaused()) {
+ boolean unPausedUponStorageWithinQuota =
+
pauseState.getReasonCode().equals(PauseState.ReasonCode.STORAGE_QUOTA_EXCEEDED);
+ if (unPausedUponStorageWithinQuota) {
+ // recreate consuming segments if table is resumed upon the table
storage getting within quota limit
+ context._recreateDeletedConsumingSegment = true;
Review Comment:
Fair enough. I was thinking of the world when the flag is not there. Maybe
it is good to remove the flag in this PR. Alternatively, you can commit this
and either one of us can remove the flag.
here is what I have for the flag removal:
- change ensureConsuming() method to be in favor of the flag always being
true
- Remove the flag from the config and the map. Anyone who deletes the
consuming segments by mistake will want to restart consumption, so it will
work. Anyone who deliberately wants to discard consuming segments can do with
the pause consumption operation command we have. If necessary, we can enhance
the pause command to optionally discard current consuming segments.
I will give a ship it, let me know if you want me to raise a PR to remove
that flag.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]