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