KKcorps commented on code in PR #19504:
URL: https://github.com/apache/pinot/pull/19504#discussion_r3987151503
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -685,10 +685,12 @@ public void replaceSegment(ImmutableSegment segment,
@Nullable ThreadSafeMutable
revertSegmentUpsertMetadata(oldSegment, segmentName,
validDocIdsForOldSegment);
return;
}
- _logger.warn("Found {} primary keys not replaced for segment: {}",
- validDocIdsForOldSegment.getCardinality(), segmentName);
- updateInconsistentRowsMetric(segmentName,
validDocIdsForOldSegment.getCardinality());
- removeSegment(oldSegment, validDocIdsForOldSegment);
+ int numKeysStillNotReplaced =
+ removeSegmentAndGetNumKeysRemoved(oldSegment,
validDocIdsForOldSegment);
+ if (numKeysStillNotReplaced > 0) {
Review Comment:
When `shouldRevertMetadataOnInconsistency` is false, we already report keys
not replaced today: the old code logs
`validDocIdsForOldSegment.getCardinality()` before calling `removeSegment()`.
This PR changes where that count comes from.
`removeSegmentAndGetNumKeysRemoved()` increments a separate counter inside
`computeIfPresent`, only when the entry still points to the old segment, before
deleting it. The returned count is preserved after cleanup; we do not read the
bitmap cardinality again here. In the OSS implementation, this cleanup also
does not clear the candidate bitmap.
For example, if the copied bitmap has two candidates but concurrent
ingestion has already moved one to a newer segment, the old code reports 2 even
though removal skips that moved key. The new code reports 1, counting only the
key that still belonged to the old segment and required removal. So reporting
remains enabled for the false branch; the change removes the stale-bitmap false
positive.
--
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]
