9aman commented on code in PR #14920:
URL: https://github.com/apache/pinot/pull/14920#discussion_r1938930531


##########
pinot-controller/src/main/java/org/apache/pinot/controller/validation/RealtimeSegmentValidationManager.java:
##########
@@ -176,6 +177,12 @@ private void runSegmentLevelValidation(TableConfig 
tableConfig) {
     // Update the total document count gauge
     _validationMetrics.updateTotalDocumentCountGauge(realtimeTableName, 
computeTotalDocumentCount(segmentsZKMetadata));
 
+    boolean isPauselessConsumptionEnabled = 
PauselessConsumptionUtils.isPauselessEnabled(tableConfig);
+
+    if (isPauselessConsumptionEnabled) {

Review Comment:
   As I see, this function is called within `runSegmentLevelValidation`. Under 
normal circumstances, this call shouldn't fetch all the segment ZK metadata as 
not many segments should be in  `ERROR`. 
   
   Moreover, segment level validations run at a reduced frequency as they fetch 
segment ZK metadata for all the segments e.g. missing download url. 
   
   These segment level validations don't impact the query or ingestion. Other 
things done by RealtimeSegmentValidationManager are ensuring that the ingestion 
works fine even in case of commit protocol failure. 
   
   I feel re-ingestion is `critical` as this will impact queries (`partial 
results` in case of `single stage` query engine and `complete failure` in case 
of `multi-stage` query engine) and hence should not be under segment level 
validations. 



-- 
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