You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/11/30 23:16:59 UTC
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7844: Make upsert inner segment update atomic
Jackie-Jiang commented on a change in pull request #7844:
URL: https://github.com/apache/pinot/pull/7844#discussion_r759734412
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartitionUpsertMetadataManager.java
##########
@@ -186,27 +182,32 @@ public GenericRow updateRecord(IndexSegment segment, RecordInfo recordInfo, Gene
}
}
+ ThreadSafeMutableRoaringBitmap validDocIds = segment.getValidDocIds();
+ assert validDocIds != null;
_result = record;
- _primaryKeyToRecordLocationMap
- .compute(hashPrimaryKey(recordInfo._primaryKey, _hashFunction), (primaryKey, currentRecordLocation) -> {
+ _primaryKeyToRecordLocationMap.compute(hashPrimaryKey(recordInfo._primaryKey, _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.
+ // 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._comparisonValue.compareTo(currentRecordLocation.getComparisonValue()) >= 0) {
IndexSegment currentSegment = currentRecordLocation.getSegment();
+ int currentDocId = currentRecordLocation.getDocId();
if (_partialUpsertHandler != null) {
// Partial upsert
_reuse.clear();
- GenericRow previousRecord = currentSegment.getRecord(currentRecordLocation.getDocId(), _reuse);
+ GenericRow previousRecord = currentSegment.getRecord(currentDocId, _reuse);
_result = _partialUpsertHandler.merge(previousRecord, record);
}
- assert currentSegment.getValidDocIds() != null;
Review comment:
`assert` is used to suppress the IDE warning on potential NPE without adding overhead. It generally means it is not possible to have `null` validDocIds. I don't think it is worth commenting though
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org