rohityadav1993 commented on code in PR #13107:
URL: https://github.com/apache/pinot/pull/13107#discussion_r1630674736
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -158,6 +158,45 @@ 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> When the replacing segment and current segment are of {@link
UploadedRealtimeSegmentName} then the PK
+ * should resolve to row in segment with higher creation time
+ * <li> For other cases resolve based on creation time of segment. In case
the creation time is same, give
+ * preference to an uploaded segment. A segment which is not LLCSegment can
be assumed to be uploaded segment and
+ * is given preference.
+ *
+ * @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) {
+
+ LLCSegmentName llcSegmentName = LLCSegmentName.of(segmentName);
+ LLCSegmentName currentLLCSegmentName =
LLCSegmentName.of(currentSegmentName);
+ if (llcSegmentName != null && currentLLCSegmentName != null) {
+ return llcSegmentName.getSequenceNumber() >
currentLLCSegmentName.getSequenceNumber();
+ }
+
+ int creationTimeComparisonRes = Long.compare(segmentCreationTimeMs,
currentSegmentCreationTimeMs);
Review Comment:
Thanks for simplifying this
> // favor the first writer
> return false;
This may be an inconsistent behaviour(there can be two writers and may not
always sync and write) but I don't have a strong inclination. It will be good
to keep it as you suggested as it does not modify the existing behaviour
For later, we can always depend on a critera based on segment metadata/name
to be consistent. e.g. we can check based on lexical order of segment name
which can enforce to use right naming conventions.
--
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
