deepthi912 commented on code in PR #19969:
URL: https://github.com/apache/pinot/pull/19969#discussion_r4373702697
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java:
##########
@@ -306,10 +306,15 @@ private void processExistingSegment(TableConfig
tableConfig, SegmentMetadata seg
"New segment crc '{}' is the same as existing segment crc for
segment '{}'. Updating ZK metadata without "
+ "refreshing the segment.", newCrc, segmentName);
// NOTE: Even though we don't need to refresh the segment, we should
still update the following fields:
- // - Creation time (not included in the crc)
+ // - Creation time (not included in the crc), except for upsert tables
// - Refresh time
// - Custom map
-
segmentZKMetadata.setCreationTime(segmentMetadata.getIndexCreationTime());
+ // Upsert servers copy the ZK creation time when they load a segment
and use it to break comparison ties.
+ // Servers don't reload a segment with the same crc, so changing it
here would let a restarted replica break
+ // ties differently from a running one.
+ if (!tableConfig.isUpsertEnabled()) {
Review Comment:
Should we change this for all the tables? Can we check the impact on what
happens if we change it for all the tables?
--
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]
