This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new dc900b5f9d1 Move partial-upsert resolveComparisonTies method from base
class to implementations (#18640)
dc900b5f9d1 is described below
commit dc900b5f9d1709a7dd8baa8d150552ff2df5b80a
Author: Chaitanya Deepthi <[email protected]>
AuthorDate: Tue Jun 2 13:07:00 2026 -0700
Move partial-upsert resolveComparisonTies method from base class to
implementations (#18640)
* Move partial-upsert resolveComparisonTies from base wrapper into
subclasses
The base addOrReplaceSegment wrapper in BasePartitionUpsertMetadataManager
existed only to call resolveComparisonTies for partial-upsert before
delegating
to the subclass-specific doAddOrReplaceSegment. This made the abstract
doAddOrReplaceSegment signature redundant with its only caller.
Inline the wrapper: callers now invoke doAddOrReplaceSegment directly, and
the
resolveComparisonTies call moves into
ConcurrentMapPartitionUpsertMetadataManager
and ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes (the
only
two non-abstract subclasses). Removes one virtual call per segment
add/replace
and eliminates a layer of indirection.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
* Fix Checkstyle line-length violation in BasePartitionUpsertMetadataManager
Line 661 was 121 chars; wrap argument list so each line stays ≤120.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
---
.../upsert/BasePartitionUpsertMetadataManager.java | 18 ++++--------------
.../ConcurrentMapPartitionUpsertMetadataManager.java | 3 +++
...itionUpsertMetadataManagerForConsistentDeletes.java | 6 ++++--
3 files changed, 11 insertions(+), 16 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
index f0c8f850b89..d78a2364186 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
@@ -494,17 +494,7 @@ public abstract class BasePartitionUpsertMetadataManager
implements PartitionUps
if (queryableDocIds == null && _deleteRecordColumn != null) {
queryableDocIds = new ThreadSafeMutableRoaringBitmap();
}
- addOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, null, null);
- }
-
- protected void addOrReplaceSegment(ImmutableSegmentImpl segment,
ThreadSafeMutableRoaringBitmap validDocIds,
- @Nullable ThreadSafeMutableRoaringBitmap queryableDocIds,
Iterator<RecordInfo> recordInfoIterator,
- @Nullable IndexSegment oldSegment, @Nullable MutableRoaringBitmap
validDocIdsForOldSegment) {
- if (_partialUpsertHandler != null) {
- recordInfoIterator = resolveComparisonTies(recordInfoIterator,
_hashFunction);
- }
- doAddOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, oldSegment,
- validDocIdsForOldSegment);
+ doAddOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, null, null);
}
protected abstract void doAddOrReplaceSegment(ImmutableSegmentImpl segment,
@@ -514,7 +504,7 @@ public abstract class BasePartitionUpsertMetadataManager
implements PartitionUps
protected void addSegmentWithoutUpsert(ImmutableSegmentImpl segment,
ThreadSafeMutableRoaringBitmap validDocIds,
@Nullable ThreadSafeMutableRoaringBitmap queryableDocIds,
Iterator<RecordInfo> recordInfoIterator) {
- addOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, null, null);
+ doAddOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, null, null);
}
/**
@@ -668,8 +658,8 @@ public abstract class BasePartitionUpsertMetadataManager
implements PartitionUps
if (queryableDocIds == null && _deleteRecordColumn != null) {
queryableDocIds = new ThreadSafeMutableRoaringBitmap();
}
- addOrReplaceSegment((ImmutableSegmentImpl) segment, validDocIds,
queryableDocIds, recordInfoIterator, oldSegment,
- validDocIdsForOldSegment);
+ doAddOrReplaceSegment((ImmutableSegmentImpl) segment, validDocIds,
queryableDocIds, recordInfoIterator,
+ oldSegment, validDocIdsForOldSegment);
}
if (_upsertViewManager != null) {
// When using consistency mode, the old segment's bitmap is updated in
place, so we get the validDocIds after
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java
index a06bd82395d..f580712576f 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java
@@ -71,6 +71,9 @@ public class ConcurrentMapPartitionUpsertMetadataManager
extends BasePartitionUp
protected void doAddOrReplaceSegment(ImmutableSegmentImpl segment,
ThreadSafeMutableRoaringBitmap validDocIds,
@Nullable ThreadSafeMutableRoaringBitmap queryableDocIds,
Iterator<RecordInfo> recordInfoIterator,
@Nullable IndexSegment oldSegment, @Nullable MutableRoaringBitmap
validDocIdsForOldSegment) {
+ if (_partialUpsertHandler != null) {
+ recordInfoIterator = resolveComparisonTies(recordInfoIterator,
_hashFunction);
+ }
String segmentName = segment.getSegmentName();
segment.enableUpsert(this, validDocIds, queryableDocIds);
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes.java
index 9a1c4f133cb..929df2ac3ac 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes.java
@@ -90,7 +90,9 @@ public class
ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes
protected void doAddOrReplaceSegment(ImmutableSegmentImpl segment,
ThreadSafeMutableRoaringBitmap validDocIds,
@Nullable ThreadSafeMutableRoaringBitmap queryableDocIds,
Iterator<RecordInfo> recordInfoIterator,
@Nullable IndexSegment oldSegment, @Nullable MutableRoaringBitmap
validDocIdsForOldSegment) {
- if (_partialUpsertHandler == null) {
+ if (_partialUpsertHandler != null) {
+ recordInfoIterator = resolveComparisonTies(recordInfoIterator,
_hashFunction);
+ } else {
// for full upsert, we are de-duping primary key once here to make sure
that we are not adding
// primary-key multiple times and subtracting just once in removeSegment.
// for partial-upsert, we call this method in base class.
@@ -298,7 +300,7 @@ public class
ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes
if (queryableDocIds == null && _deleteRecordColumn != null) {
queryableDocIds = new ThreadSafeMutableRoaringBitmap();
}
- addOrReplaceSegment((ImmutableSegmentImpl) segment, validDocIds,
queryableDocIds, recordInfoIterator,
+ doAddOrReplaceSegment((ImmutableSegmentImpl) segment, validDocIds,
queryableDocIds, recordInfoIterator,
oldSegment, validDocIdsForOldSegment);
}
if (validDocIdsForOldSegment != null &&
!validDocIdsForOldSegment.isEmpty()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
