[GitHub] [hadoop] vinayakumarb commented on a change in pull request #4030: HADOOP-18143. toString method of RpcCall throws IllegalArgumentException

[GitBox <gi...@apache.org>]

vinayakumarb commented on a change in pull request #4030:
URL: https://github.com/apache/hadoop/pull/4030#discussion_r818078733



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufRpcEngine2.java
##########
@@ -629,6 +630,7 @@ private RpcWritable call(RPC.Server server,
   // which uses the rpc header.  in the normal case we want to defer decoding
   // the rpc header until needed by the rpc engine.
   static class RpcProtobufRequest extends RpcWritable.Buffer {
+    private final ReentrantLock lock = new ReentrantLock();

Review comment:
       I guess re-entrant lock is unnecessary.
   adding `synchronized` to `getRequestHeader()` method and calling this method instead of all direct usages of `requestHeader` would be sufficient. 
   This would ensure that `requestHeader` will never be null. So unnecessary null check can be removed.
   
   Remember, this is a critical path, any extra performance impacts, however small it is would impact the overall RPC performance.




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