jackjlli commented on a change in pull request #7427:
URL: https://github.com/apache/pinot/pull/7427#discussion_r708690584
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java
##########
@@ -175,10 +189,12 @@ private void processExistingSegment(SegmentMetadata
segmentMetadata, URI finalSe
}
_pinotHelixResourceManager
- .refreshSegment(tableNameWithType, segmentMetadata,
existingSegmentZKMetadata, zkDownloadURI, crypter);
+ .refreshSegment(tableNameWithType, segmentMetadata,
existingSegmentZKMetadata,
+ replaceOnly ? expectedVersion : -1, zkDownloadURI, crypter);
Review comment:
It seems this code `replaceOnly ? expectedVersion : -1` appears in
multiple places. Is it possible to do only one check and then pass this final
value to the target places?
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java
##########
@@ -118,10 +127,12 @@ private void processExistingSegment(SegmentMetadata
segmentMetadata, URI finalSe
// Lock the segment by setting the upload start time in ZK
existingSegmentZKMetadata.setSegmentUploadStartTime(System.currentTimeMillis());
if (!_pinotHelixResourceManager
- .updateZkMetadata(tableNameWithType, existingSegmentZKMetadata,
znRecord.getVersion())) {
+ .updateZkMetadata(tableNameWithType, existingSegmentZKMetadata,
expectedVersion)) {
throw new ControllerApplicationException(LOGGER,
"Failed to lock the segment: " + segmentName + " of table: " +
tableNameWithType + ", retry later",
Response.Status.CONFLICT);
+ } else {
+ expectedVersion++;
Review comment:
Could you add some comment on why the expected version needs to self
increase, and why it only increases by 1?
--
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]
