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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to