abhishekagarwal87 commented on a change in pull request #11398:
URL: https://github.com/apache/druid/pull/11398#discussion_r668647828



##########
File path: 
server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java
##########
@@ -154,7 +166,8 @@ public synchronized File reserve(String 
segmentFilePathToAdd, String segmentId,
   {
     final File segmentFileToAdd = new File(path, segmentFilePathToAdd);
     if (files.contains(segmentFileToAdd)) {
-      return null;
+      //TODO: is this change ok?
+      return segmentFileToAdd;

Review comment:
       a `null` result would imply that the reservation has actually failed 
because the storage location can't handle the file size and that's how it is 
interpreted outside. But if the file is already reserved, the operation didn't 
technically fail. With this change, the method can be called multiple times 
with correct semantics. 




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