[GitHub] [hadoop] omalley commented on a diff in pull request #5371: HDFS-18324. Fix race condition in closing IPC connections.

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

omalley commented on code in PR #5371:
URL: https://github.com/apache/hadoop/pull/5371#discussion_r1100757833


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -1159,29 +1159,35 @@ public void run() {
      */
     public void sendRpcRequest(final Call call)
         throws InterruptedException, IOException {
-      if (shouldCloseConnection.get()) {
-        return;
-      }
-
-      // Serialize the call to be sent. This is done from the actual
-      // caller thread, rather than the rpcRequestThread in the connection,
-      // so that if the serialization throws an error, it is reported
-      // properly. This also parallelizes the serialization.
-      //
-      // Format of a call on the wire:
-      // 0) Length of rest below (1 + 2)
-      // 1) RpcRequestHeader  - is serialized Delimited hence contains length
-      // 2) RpcRequest
-      //
-      // Items '1' and '2' are prepared here. 
-      RpcRequestHeaderProto header = ProtoUtil.makeRpcRequestHeader(
-          call.rpcKind, OperationProto.RPC_FINAL_PACKET, call.id, call.retry,
-          clientId, call.alignmentContext);
+      if (!shouldCloseConnection.get()) {

Review Comment:
   We can. Originally, I had the end of the method throw an exception, but I decided that was too dangerous a change from the current semantics. (Although I find that ignoring the failure to send the request an anti-pattern. It looks like the calling code in Client.call was expecting a RejectedExecutionException from sendRpcRequest if the connection was closed, but that must have been vestigial from long ago. 



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