tibrewalpratik17 commented on code in PR #13285:
URL: https://github.com/apache/pinot/pull/13285#discussion_r1631691980


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -832,8 +836,10 @@ public void takeSnapshot() {
     if (!_enableSnapshot) {
       return;
     }
-    if (!_gotFirstConsumingSegment) {
-      _logger.info("Skip taking snapshot before getting the first consuming 
segment");
+    if (_partialUpsertHandler == null && !_gotFirstConsumingSegment) {

Review Comment:
   Couple of reasons for this:
   - One is if there is no active consumption in a partition but you enable 
snapshotting then we never get to snapshot the previous segments after server 
restarts. Related issue - 
https://github.com/apache/pinot/issues/12703#issuecomment-2154850044
   - Other is now when we do a restart / rollout in a cluster, it becomes a one 
day waiting time for us to have any segments compacted. The scale of deletion 
is pretty huge (~600M records per day) and so imo it's a good optimisation to 
have for making compaction work early. 



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