[GitHub] [hadoop] steveloughran commented on a diff in pull request #5162: HDFS-16853. BugFix HADOOP-18324 caused UT TestLeaseRecovery2#testHardLeaseRecoveryAfterNameNodeRestart failed

["steveloughran (via GitHub)" <gi...@apache.org>]

steveloughran commented on code in PR #5162:
URL: https://github.com/apache/hadoop/pull/5162#discussion_r1093579485


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -1182,7 +1183,12 @@ public void sendRpcRequest(final Call call)
       final ResponseBuffer buf = new ResponseBuffer();
       header.writeDelimitedTo(buf);
       RpcWritable.wrap(call.rpcRequest).writeTo(buf);
-      rpcRequestQueue.put(Pair.of(call, buf));
+      synchronized (sendRpcRequestLock) {

Review Comment:
   markClosed doesn't use the same lock, so i don't see this stopping a race condition between
   this submission and the markClosed set+purge queue;
   
   better: move rpcRequestQueue.put(Pair.of(call, buf)); into a new method, and make synchronized
   
   ```java
   private sychronized put(pair) {
   if (!shouldCloseConnection.get()) {
        rpcRequestQueue.put(pair);
     }
   }
   ```
   
   



-- 
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-unsubscribe@hadoop.apache.org

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


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