virajjasani commented on a change in pull request #3719:
URL: https://github.com/apache/hadoop/pull/3719#discussion_r762454550



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
##########
@@ -1530,15 +1530,30 @@ private void closeCurrentConnection(SelectionKey key, 
Throwable e) {
     InetSocketAddress getAddress() {
       return (InetSocketAddress)acceptChannel.socket().getLocalSocketAddress();
     }
-    
+
+    protected void configureSocketChannel(SocketChannel channel) throws 
IOException {

Review comment:
       Same here, good to annotate with IA.Private and VFT

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
##########
@@ -1325,7 +1325,7 @@ public String toString() {
   }
 
   /** Listens on the socket. Creates jobs for the handler threads*/
-  private class Listener extends Thread {
+  protected class Listener extends Thread {

Review comment:
       Would have been better if we could keep this private, but it seems we 
have no better way if we want to test this change (Thanks for the test btw). If 
we are going to go forward with this, we should annotate this as 
`@VisibleForTesting` and `@InterfaceAudience.Private`.
   
   Let's also get some expert advice on this. cc @aajisaka @ayushtkn 
@jojochuang 




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