rohityadav1993 commented on code in PR #13107:
URL: https://github.com/apache/pinot/pull/13107#discussion_r1631688317
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -158,6 +158,47 @@ protected void addOrReplaceSegment(ImmutableSegmentImpl
segment, ThreadSafeMutab
}
}
+ /**
+ * <li> When the replacing segment and current segment are of {@link
LLCSegmentName} then the PK should resolve to
+ * row in segment with higher sequence id.
+ * <li> If either or both are not LLC segment, then resolve based on
creation time of segment. If creation time is
+ * same then prefer uploaded segment if other is LLCSegmentName
+ * <li> If both are uploaded segment, prefer standard
UploadedRealtimeSegmentName, if still a tie, then resolve to
+ * current segment.
+ *
+ * @param segmentName replacing segment name
+ * @param currentSegmentName current segment name having the record for the
given primary key
+ * @param segmentCreationTimeMs replacing segment creation time
+ * @param currentSegmentCreationTimeMs current segment creation time
+ * @return true if the record in replacing segment should replace the record
in current segment
+ */
+ protected boolean shouldReplaceOnComparisonTie(String segmentName, String
currentSegmentName,
+ long segmentCreationTimeMs, long currentSegmentCreationTimeMs) {
+
+ // resolve using sequence id if both are LLCSegmentName
+ LLCSegmentName llcSegmentName = LLCSegmentName.of(segmentName);
+ LLCSegmentName currentLLCSegmentName =
LLCSegmentName.of(currentSegmentName);
+ if (llcSegmentName != null && currentLLCSegmentName != null) {
+ return llcSegmentName.getSequenceNumber() >
currentLLCSegmentName.getSequenceNumber();
+ }
+
+ // either or both are uploaded segments, prefer the latest segment
+ int creationTimeComparisonRes = Long.compare(segmentCreationTimeMs,
currentSegmentCreationTimeMs);
+ if ((llcSegmentName == null || currentLLCSegmentName == null) &&
creationTimeComparisonRes != 0) {
Review Comment:
This is true, tried to be extra carefull and this was the side effect,
Removed this and tested with UTs. The behaviour is as expected. PTAL at the
UTs as well. Updating.
--
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
