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
commit cycle 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 and even have concise
snapshots.
--
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
