vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597551434


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##########
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src, 
INodesInPath iip,
       NameNode.stateChangeLog.warn("BLOCK*" +
           " internalReleaseLease: All existing blocks are COMPLETE," +
           " lease removed, file " + src + " closed.");
-      return true;  // closed!
+      return false;  // closed!

Review Comment:
   As per javadoc of this method, return value indicates whether file was 
closed or not.
   
   Changing that value here, may solve the problem of logSync() particularly in 
this case, but it will be problematic for other usages of this method.
   
   For ex: recoverLease() RPC will always get false, even though file was 
closed.
   
   As per the javadoc, even if the return value is false, there are edits 
logged (reassigning the lease, when blockrecovery is initiated.).
   So calling the logSync() is required in both these cases.
   That said, Cannot blindly call logSync() always.
   
   So, more correct approach to fix this is to return a combination of these 
values from this method (i.e. complerted and needsync )
   
   And determine whether to call sync or not in the caller.



-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to