Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-14771

RpcServer#getRemoteAddress always returns null

Details

    • Bug
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 1.2.0
    • 1.2.0, 1.3.0, 0.98.17, 2.0.0
    • IPC/RPC
    • None
    • Reviewed

    Description

      RpcServer.getRemoteAddress always returns null, because Call object is getting initialized with null.This seems to be happening because of using RpcServer.getRemoteIp() in Call object constructor before RpcServer thread local 'CurCall' being set in CallRunner.run method:

      // --- RpcServer.java ---
      protected void processRequest(byte[] buf) throws IOException, InterruptedException {
       .................................
      // Call object getting initialized here with address 
      // obtained from RpcServer.getRemoteIp()
      Call call = new Call(id, this.service, md, header, param, cellScanner, this, responder,
                    totalRequestSize, traceInfo, RpcServer.getRemoteIp());
        scheduler.dispatch(new CallRunner(RpcServer.this, call));
       }
      
      // getRemoteIp method gets address from threadlocal 'CurCall' which 
      // gets set in CallRunner.run and calling it before this as in above case, will return null
      // --- CallRunner.java ---
      public void run() {
        .........................   
        Pair<Message, CellScanner> resultPair = null;
        RpcServer.CurCall.set(call);
        ..............................
      }
      
      // Using 'this.addr' in place of getRemoteIp method in RpcServer.java seems to be fixing this issue
      Call call = new Call(id, this.service, md, header, param, cellScanner, this, responder,
                    totalRequestSize, traceInfo, this.addr);
      

      Attachments

        1. 14771-V2.patch
          7 kB
          Ted Yu
        2. HBASE-14771.patch
          1 kB
          Abhishek Kumar
        3. HBASE-14771-addendum-0.98.patch
          1 kB
          Andrew Kyle Purtell
        4. HBASE-14771-V1.patch
          4 kB
          Abhishek Kumar
        5. HBASE-14771-V2.patch
          7 kB
          Abhishek Kumar

        Activity

          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #1161 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/1161/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 8189b0f255023c6d2e10565275979cea23f87e7d)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #1161 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/1161/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 8189b0f255023c6d2e10565275979cea23f87e7d) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-matrix #287 (See https://builds.apache.org/job/HBase-0.98-matrix/287/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 8189b0f255023c6d2e10565275979cea23f87e7d)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-matrix #287 (See https://builds.apache.org/job/HBase-0.98-matrix/287/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 8189b0f255023c6d2e10565275979cea23f87e7d) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-Trunk_matrix #638 (See https://builds.apache.org/job/HBase-Trunk_matrix/638/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 79588240c60bf422fb9d9a74e0edd8b47a66b9f9)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-Trunk_matrix #638 (See https://builds.apache.org/job/HBase-Trunk_matrix/638/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 79588240c60bf422fb9d9a74e0edd8b47a66b9f9) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-1.2-IT #398 (See https://builds.apache.org/job/HBase-1.2-IT/398/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 063d214f91e93b0057416e6723de406b3fcbfe35)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.2-IT #398 (See https://builds.apache.org/job/HBase-1.2-IT/398/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 063d214f91e93b0057416e6723de406b3fcbfe35) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-1.3-IT #440 (See https://builds.apache.org/job/HBase-1.3-IT/440/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 1f1a8fd7d3f80bd98bae1ed3b604b4bdb692c1de)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.3-IT #440 (See https://builds.apache.org/job/HBase-1.3-IT/440/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 1f1a8fd7d3f80bd98bae1ed3b604b4bdb692c1de) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.3 #497 (See https://builds.apache.org/job/HBase-1.3/497/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 1f1a8fd7d3f80bd98bae1ed3b604b4bdb692c1de)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.3 #497 (See https://builds.apache.org/job/HBase-1.3/497/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 1f1a8fd7d3f80bd98bae1ed3b604b4bdb692c1de) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-1.2 #507 (See https://builds.apache.org/job/HBase-1.2/507/)
          Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 063d214f91e93b0057416e6723de406b3fcbfe35)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.2 #507 (See https://builds.apache.org/job/HBase-1.2/507/ ) Amend HBASE-14771 RpcServer#getRemoteAddress always returns null (apurtell: rev 063d214f91e93b0057416e6723de406b3fcbfe35) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java

          Committed the addendum to 0.98, branch-1.2, branch-1, and master

          apurtell Andrew Kyle Purtell added a comment - Committed the addendum to 0.98, branch-1.2, branch-1, and master

          Addendum adds a null check for 'call.connection' as well. Patch based on 0.98 but the change is the same everywhere.

          apurtell Andrew Kyle Purtell added a comment - Addendum adds a null check for 'call.connection' as well. Patch based on 0.98 but the change is the same everywhere.

          This change will cause Phoenix's PhoenixIndexRpcSchedulerTest to fail with a NPE. There is something very simple (and correct) we can do as an addendum to fix it. Attaching the addendum. Unless objection will commit the addendum to all branches that have this change shortly, to get the 0.98 RC out the door.

          apurtell Andrew Kyle Purtell added a comment - This change will cause Phoenix's PhoenixIndexRpcSchedulerTest to fail with a NPE. There is something very simple (and correct) we can do as an addendum to fix it. Attaching the addendum. Unless objection will commit the addendum to all branches that have this change shortly, to get the 0.98 RC out the door.
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-matrix #264 (See https://builds.apache.org/job/HBase-0.98-matrix/264/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (apurtell: rev 17c7b0c6aca705cab6ff054128eedf490beb9886)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestIPC.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-matrix #264 (See https://builds.apache.org/job/HBase-0.98-matrix/264/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (apurtell: rev 17c7b0c6aca705cab6ff054128eedf490beb9886) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestIPC.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #1137 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/1137/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (apurtell: rev 17c7b0c6aca705cab6ff054128eedf490beb9886)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestIPC.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #1137 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/1137/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (apurtell: rev 17c7b0c6aca705cab6ff054128eedf490beb9886) hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestIPC.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-Trunk_matrix #477 (See https://builds.apache.org/job/HBase-Trunk_matrix/477/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 1b13bfcd43862451a807fa0f1943de781ee8888f)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-Trunk_matrix #477 (See https://builds.apache.org/job/HBase-Trunk_matrix/477/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 1b13bfcd43862451a807fa0f1943de781ee8888f) hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-1.3-IT #319 (See https://builds.apache.org/job/HBase-1.3-IT/319/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 5362ac03116c9b44f0e82a3d69f194f2fc467e51)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.3-IT #319 (See https://builds.apache.org/job/HBase-1.3-IT/319/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 5362ac03116c9b44f0e82a3d69f194f2fc467e51) hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.2 #380 (See https://builds.apache.org/job/HBase-1.2/380/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev e365be4bc3f18ba7ba4abbcbb6fab0117d457562)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.2 #380 (See https://builds.apache.org/job/HBase-1.2/380/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev e365be4bc3f18ba7ba4abbcbb6fab0117d457562) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.3 #376 (See https://builds.apache.org/job/HBase-1.3/376/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 5362ac03116c9b44f0e82a3d69f194f2fc467e51)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.3 #376 (See https://builds.apache.org/job/HBase-1.3/376/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev 5362ac03116c9b44f0e82a3d69f194f2fc467e51) hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-1.2-IT #288 (See https://builds.apache.org/job/HBase-1.2-IT/288/)
          HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev e365be4bc3f18ba7ba4abbcbb6fab0117d457562)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.2-IT #288 (See https://builds.apache.org/job/HBase-1.2-IT/288/ ) HBASE-14771 RpcServer#getRemoteAddress always returns null (Abhishek (tedyu: rev e365be4bc3f18ba7ba4abbcbb6fab0117d457562) hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
          yuzhihong@gmail.com Ted Yu added a comment -

          Thanks for the patch, Abhishek

          yuzhihong@gmail.com Ted Yu added a comment - Thanks for the patch, Abhishek
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12772836/14771-V2.patch
          against master branch at commit dadfe7da0484be81ae09ad61f976967b9893c38d.
          ATTACHMENT ID: 12772836

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 protoc. The applied patch does not increase the total number of protoc compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn post-site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12772836/14771-V2.patch against master branch at commit dadfe7da0484be81ae09ad61f976967b9893c38d. ATTACHMENT ID: 12772836 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn post-site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16555//console This message is automatically generated.
          yuzhihong@gmail.com Ted Yu added a comment -

          Re-attaching patch v2

          yuzhihong@gmail.com Ted Yu added a comment - Re-attaching patch v2
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12772116/HBASE-14771-V2.patch
          against master branch at commit 789f8a5a70242c16ce10bc95401c51c7d04debfa.
          ATTACHMENT ID: 12772116

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 protoc. The applied patch does not increase the total number of protoc compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn post-site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12772116/HBASE-14771-V2.patch against master branch at commit 789f8a5a70242c16ce10bc95401c51c7d04debfa. ATTACHMENT ID: 12772116 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn post-site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16515//console This message is automatically generated.

          +1

          appy Apekshit Sharma added a comment - +1
          a72877 Abhishek Kumar added a comment -

          i was on leave for two days, updated patch attached, thanks.

          a72877 Abhishek Kumar added a comment - i was on leave for two days , updated patch attached, thanks.

          a72877 Let's drive it to the end. Only need to copy-paste the code from your last comment.

          appy Apekshit Sharma added a comment - a72877 Let's drive it to the end. Only need to copy-paste the code from your last comment.

          Perfect!

          appy Apekshit Sharma added a comment - Perfect!
          a72877 Abhishek Kumar added a comment -

          thanks Appy for review, i m planning to write something like below which returns remoteAddress from 'echo' method back to client, pls let me know if it seems okay for test:

          // in static nested class TestRpcServer1
          TestRpcServer1(RpcScheduler scheduler) throws IOException {
                super(null, "testRemoteAddressInCallObject", Lists
                    .newArrayList(new BlockingServiceAndInterface(TestRpcServiceProtos.TestProtobufRpcProto
                        .newReflectiveBlockingService(SERVICE1), null)),
                    new InetSocketAddress("localhost", 0), CONF, scheduler);
              }
          
          // echo message return remoteAddress 
          @Override
             public EchoResponseProto echo(RpcController unused, EchoRequestProto request)
                  throws ServiceException {
                final InetAddress remoteAddr = TestRpcServer1.getRemoteAddress();
                final String message = remoteAddr == null ? "NULL" : remoteAddr.getHostAddress();
                return EchoResponseProto.newBuilder().setMessage(message).build();
               }
          
          // in test method
           @Test
            public void testRpcServerForNotNullRemoteAddressInCallObject() throws IOException,
                ServiceException {
              final RpcScheduler scheduler = new FifoRpcScheduler(CONF, 1);
              final TestRpcServer1 rpcServer = new TestRpcServer1(scheduler);
              final InetSocketAddress localAddr = new InetSocketAddress("localhost", 0);
              final AbstractRpcClient client =
                  new RpcClientImpl(CONF, HConstants.CLUSTER_ID_DEFAULT, localAddr, null);
              try {
                rpcServer.start();
                final InetSocketAddress isa = rpcServer.getListenerAddress();
                if (isa == null) {
                  throw new IOException("Listener channel is closed");
                }
                final BlockingRpcChannel channel =
                    client.createBlockingRpcChannel(
                      ServerName.valueOf(isa.getHostName(), isa.getPort(), System.currentTimeMillis()),
                      User.getCurrent(), 0);
                TestRpcServiceProtos.TestProtobufRpcProto.BlockingInterface stub =
                    TestRpcServiceProtos.TestProtobufRpcProto.newBlockingStub(channel);
                final EchoRequestProto echoRequest =
                    EchoRequestProto.newBuilder().setMessage("GetRemoteAddress").build();
                final EchoResponseProto echoResponse = stub.echo(null, echoRequest);
                Assert.assertEquals(localAddr.getAddress().getHostAddress(), echoResponse.getMessage());
              } finally {
                client.close();
                rpcServer.stop();
              }
            }
          
          
          a72877 Abhishek Kumar added a comment - thanks Appy for review, i m planning to write something like below which returns remoteAddress from 'echo' method back to client, pls let me know if it seems okay for test: // in static nested class TestRpcServer1 TestRpcServer1(RpcScheduler scheduler) throws IOException { super(null, "testRemoteAddressInCallObject", Lists .newArrayList(new BlockingServiceAndInterface(TestRpcServiceProtos.TestProtobufRpcProto .newReflectiveBlockingService(SERVICE1), null)), new InetSocketAddress("localhost", 0), CONF, scheduler); } // echo message return remoteAddress @Override public EchoResponseProto echo(RpcController unused, EchoRequestProto request) throws ServiceException { final InetAddress remoteAddr = TestRpcServer1.getRemoteAddress(); final String message = remoteAddr == null ? "NULL" : remoteAddr.getHostAddress(); return EchoResponseProto.newBuilder().setMessage(message).build(); } // in test method @Test public void testRpcServerForNotNullRemoteAddressInCallObject() throws IOException, ServiceException { final RpcScheduler scheduler = new FifoRpcScheduler(CONF, 1); final TestRpcServer1 rpcServer = new TestRpcServer1(scheduler); final InetSocketAddress localAddr = new InetSocketAddress("localhost", 0); final AbstractRpcClient client = new RpcClientImpl(CONF, HConstants.CLUSTER_ID_DEFAULT, localAddr, null); try { rpcServer.start(); final InetSocketAddress isa = rpcServer.getListenerAddress(); if (isa == null) { throw new IOException("Listener channel is closed"); } final BlockingRpcChannel channel = client.createBlockingRpcChannel( ServerName.valueOf(isa.getHostName(), isa.getPort(), System.currentTimeMillis()), User.getCurrent(), 0); TestRpcServiceProtos.TestProtobufRpcProto.BlockingInterface stub = TestRpcServiceProtos.TestProtobufRpcProto.newBlockingStub(channel); final EchoRequestProto echoRequest = EchoRequestProto.newBuilder().setMessage("GetRemoteAddress").build(); final EchoResponseProto echoResponse = stub.echo(null, echoRequest); Assert.assertEquals(localAddr.getAddress().getHostAddress(), echoResponse.getMessage()); } finally { client.close(); rpcServer.stop(); } }

          Thanks a72877 for adding the test.
          So we check that's it's not null, but am a bit confused here, what is the real value of address here since "AbstractRpcClient client = createRpcClient(conf);" creates client with localAddr as null.

          Also, tests which do not change functionality of non-test code (FifoScheduler here) are better ones. The added test is only checking that Call.remoteAddress is set correctly, but if we check that RpcServer.getRemoteAddress works correctly, it'll cover the former case and also will be a more robust test of functionality.
          So how about trying this..change ping method to return value of Rpc.getRemoteAddress and check it's equal to some expected address.

          appy Apekshit Sharma added a comment - Thanks a72877 for adding the test. So we check that's it's not null, but am a bit confused here, what is the real value of address here since "AbstractRpcClient client = createRpcClient(conf);" creates client with localAddr as null. Also, tests which do not change functionality of non-test code (FifoScheduler here) are better ones. The added test is only checking that Call.remoteAddress is set correctly, but if we check that RpcServer.getRemoteAddress works correctly, it'll cover the former case and also will be a more robust test of functionality. So how about trying this..change ping method to return value of Rpc.getRemoteAddress and check it's equal to some expected address.
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12771025/HBASE-14771-V1.patch
          against master branch at commit bfa36891901b96b95d82f5307642c35fd2b9f534.
          ATTACHMENT ID: 12771025

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 protoc. The applied patch does not increase the total number of protoc compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn post-site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12771025/HBASE-14771-V1.patch against master branch at commit bfa36891901b96b95d82f5307642c35fd2b9f534. ATTACHMENT ID: 12771025 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn post-site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16432//console This message is automatically generated.
          a72877 Abhishek Kumar added a comment -

          Test case added, pls review the same.

          a72877 Abhishek Kumar added a comment - Test case added, pls review the same.

          the patch looks good. Can you think of a unit test? Maybe using dummy service and method.

          appy Apekshit Sharma added a comment - the patch looks good. Can you think of a unit test? Maybe using dummy service and method.
          a72877 Abhishek Kumar added a comment -

          it was mainly for logging purpose such as the use of HMaster.getClientIdAuditPrefix() in create/modify etc operations and we have some custom co-processor also making use of this getRemoteAddress method which was causing some issues for us.

          a72877 Abhishek Kumar added a comment - it was mainly for logging purpose such as the use of HMaster.getClientIdAuditPrefix() in create/modify etc operations and we have some custom co-processor also making use of this getRemoteAddress method which was causing some issues for us.

          getRemoteAddress() always returns null because CurCall is thread local variable and is only set in threads responsible for executing CallRunners (executor in FifoRpcScheduler and those being created in RpcExecutor.startHandlers).
          RpcServer.getRemoteIp also uses CurCall, so it'll always return null here. Using

          this.addr

          makes sense.
          It'll definitely fix 2 of the 3 uses of getRemoteAddress(). I am not sure about the use in AccessController since I don't know it's internals.
          a72877 what do you mean by "... fixing this issue"? Where are you seeing the null values which change to legitimate address after this change?

          appy Apekshit Sharma added a comment - getRemoteAddress() always returns null because CurCall is thread local variable and is only set in threads responsible for executing CallRunners (executor in FifoRpcScheduler and those being created in RpcExecutor.startHandlers). RpcServer.getRemoteIp also uses CurCall, so it'll always return null here. Using this .addr makes sense. It'll definitely fix 2 of the 3 uses of getRemoteAddress(). I am not sure about the use in AccessController since I don't know it's internals. a72877 what do you mean by "... fixing this issue"? Where are you seeing the null values which change to legitimate address after this change?
          stack Michael Stack added a comment -

          Good one. Hmmm. This used to work. Stuff got moved around? Do you know appy ? Can you see? Is this the only place a Call is made? There are a few others if only on error. We should set address there too? Nice one.

          stack Michael Stack added a comment - Good one. Hmmm. This used to work. Stuff got moved around? Do you know appy ? Can you see? Is this the only place a Call is made? There are a few others if only on error. We should set address there too? Nice one.
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12770801/HBASE-14771.patch
          against master branch at commit 050ebe850b32057860fb94b46f955352db139db1.
          ATTACHMENT ID: 12770801

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 protoc. The applied patch does not increase the total number of protoc compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn post-site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12770801/HBASE-14771.patch against master branch at commit 050ebe850b32057860fb94b46f955352db139db1. ATTACHMENT ID: 12770801 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn post-site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16409//console This message is automatically generated.
          a72877 Abhishek Kumar added a comment -

          patch attached , pls review the same, thanks

          a72877 Abhishek Kumar added a comment - patch attached , pls review the same, thanks

          People

            a72877 Abhishek Kumar
            a72877 Abhishek Kumar
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: