[GitHub] [pinot] jackjlli commented on a diff in pull request #8667: Handle the race condition for common dest segments in startReplaceSegments()

[GitBox]

jackjlli commented on code in PR #8667:
URL: https://github.com/apache/pinot/pull/8667#discussion_r869674942


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3132,8 +3132,20 @@ public String startReplaceSegments(String 
tableNameWithType, List<String> segmen
               // Update segment lineage entry to 'REVERTED'
               updateSegmentLineageEntryToReverted(tableNameWithType, 
segmentLineage, entryId, lineageEntry);
 
+              // Update segmentsTo of the 'REVERTED' entry to handle the 
following case:

Review Comment:
   I don't think we can directly remove the `IN_PROGRESS` entry, otherwise any 
segments in `segmentsTo` can be queried in pinot brokers, while the next batch 
of segment upload is still in progress.
   E.g. if the 1st lineage entry is like `{segmentsTo: [s3, s4]}` and the next 
one is `{segmentsTo: [s4, s5]}`, then s3 can be queried once the lineage is 
removed. So only when s3 is removed can we safely delete the old `IN_PROGRESS` 
entry.



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