Jackie-Jiang commented on code in PR #10035:
URL: https://github.com/apache/pinot/pull/10035#discussion_r1057296261


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -216,6 +216,29 @@ public void addRecord(MutableSegment segment, RecordInfo 
recordInfo) {
         _primaryKeyToRecordLocationMap.size());
   }
 
+  @Override
+  public void removeRecord(RecordInfo recordInfo) {
+    
_primaryKeyToRecordLocationMap.compute(HashUtils.hashPrimaryKey(recordInfo.getPrimaryKey(),
 _hashFunction),
+        (primaryKey, currentRecordLocation) -> {
+          if (currentRecordLocation != null) {
+            // Existing primary key
+
+            // Update the record location when the new comparison value is 
greater than or equal to the current value.
+            // Update the record location when there is a tie to keep the 
newer record.
+            if 
(recordInfo.getComparisonValue().compareTo(currentRecordLocation.getComparisonValue())
 >= 0) {
+              IndexSegment currentSegment = currentRecordLocation.getSegment();
+              int currentDocId = currentRecordLocation.getDocId();
+              
Objects.requireNonNull(currentSegment.getValidDocIds()).remove(currentDocId);
+              return null;
+            } else {
+              return currentRecordLocation;
+            }
+          } else {
+            return null;
+          }
+        });

Review Comment:
   Update the gauge here



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java:
##########
@@ -162,6 +163,8 @@ public class MutableSegmentImpl implements MutableSegment {
   private RealtimeLuceneIndexRefreshState.RealtimeLuceneReaders 
_realtimeLuceneReaders;
 
   private final UpsertConfig.Mode _upsertMode;
+  private final boolean _enableDeleteInUpsert;

Review Comment:
   Consider removing this config, and enable deletion whenever 
`_upsertDeleteKey` is configured



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -216,6 +216,29 @@ public void addRecord(MutableSegment segment, RecordInfo 
recordInfo) {
         _primaryKeyToRecordLocationMap.size());
   }
 
+  @Override
+  public void removeRecord(RecordInfo recordInfo) {
+    
_primaryKeyToRecordLocationMap.compute(HashUtils.hashPrimaryKey(recordInfo.getPrimaryKey(),
 _hashFunction),
+        (primaryKey, currentRecordLocation) -> {
+          if (currentRecordLocation != null) {
+            // Existing primary key
+
+            // Update the record location when the new comparison value is 
greater than or equal to the current value.

Review Comment:
   Update the comment



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -216,6 +216,29 @@ public void addRecord(MutableSegment segment, RecordInfo 
recordInfo) {
         _primaryKeyToRecordLocationMap.size());
   }
 
+  @Override
+  public void removeRecord(RecordInfo recordInfo) {
+    
_primaryKeyToRecordLocationMap.compute(HashUtils.hashPrimaryKey(recordInfo.getPrimaryKey(),
 _hashFunction),
+        (primaryKey, currentRecordLocation) -> {
+          if (currentRecordLocation != null) {
+            // Existing primary key
+
+            // Update the record location when the new comparison value is 
greater than or equal to the current value.
+            // Update the record location when there is a tie to keep the 
newer record.
+            if 
(recordInfo.getComparisonValue().compareTo(currentRecordLocation.getComparisonValue())
 >= 0) {
+              IndexSegment currentSegment = currentRecordLocation.getSegment();
+              int currentDocId = currentRecordLocation.getDocId();
+              
Objects.requireNonNull(currentSegment.getValidDocIds()).remove(currentDocId);
+              return null;
+            } else {
+              return currentRecordLocation;
+            }
+          } else {
+            return null;

Review Comment:
   Consider putting a warning here since the record cannot be found



-- 
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]

Reply via email to