Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-11861

ipc.Client.Connection#sendRpcRequest should log request name

    Details

      Description

      ipc.Client.Connection#sendRpcRequest only logs the call id.

                      if (LOG.isDebugEnabled())
                        LOG.debug(getName() + " sending #" + call.id);
      

      It'd be much more helpful to log request name for several benefits:

      • Find out which requests sent to which target
      • Correlate with the debug log in ipc.Server.Handler

        Issue Links

          Activity

          Hide
          jzhuge John Zhuge added a comment - - edited

          Any IPC call without parameter will leave parameters as null, then NPE whenever toString is called.

          The reason why trunk does not have the issue:

          • For TestClientToAMTokens, YARN-5163. "Migrate TestClientToAMTokens and TestClientRMTokens tests from the old RPC engine. " replaced all client.ping() with client.ping(null, TestRpcBase.newEmptyRequest()).
          • TestClientProtocolWithDelegationToken is removed in trunk

          Filed HADOOP-14511 "WritableRpcEngine.Invocation#toString NPE on null parameters", and submitted patch 001.

          Show
          jzhuge John Zhuge added a comment - - edited Any IPC call without parameter will leave parameters as null, then NPE whenever toString is called. The reason why trunk does not have the issue: For TestClientToAMTokens, YARN-5163 . "Migrate TestClientToAMTokens and TestClientRMTokens tests from the old RPC engine. " replaced all client.ping() with client.ping(null, TestRpcBase.newEmptyRequest()) . TestClientProtocolWithDelegationToken is removed in trunk Filed HADOOP-14511 "WritableRpcEngine.Invocation#toString NPE on null parameters", and submitted patch 001.
          Hide
          jzhuge John Zhuge added a comment -

          Thanks for the report Eric Badger. Reproduced the test failure on branch-2 but not on trunk.

          The new debug code tries to print call.rpcRequest but its parameters field is still null at this point at least for these test cases:

          WritableRpcEngine.Invocation#toString
                for (int i = 0; i < parameters.length; i++) {
                  if (i != 0)
                    buffer.append(", ");
                  buffer.append(parameters[i]);
                }
          

          https://github.com/apache/hadoop/blob/2448d845723b1a52336481458c7d9582bcb3714a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java#L185

          This looks like a bug in WritableRpcEngine.Invocation#toString unless rpcRequest is guaranteed to be not null. Still puzzled why the test case did not fail on trunk.

          Show
          jzhuge John Zhuge added a comment - Thanks for the report Eric Badger . Reproduced the test failure on branch-2 but not on trunk. The new debug code tries to print call.rpcRequest but its parameters field is still null at this point at least for these test cases: WritableRpcEngine.Invocation#toString for ( int i = 0; i < parameters.length; i++) { if (i != 0) buffer.append( ", " ); buffer.append(parameters[i]); } https://github.com/apache/hadoop/blob/2448d845723b1a52336481458c7d9582bcb3714a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java#L185 This looks like a bug in WritableRpcEngine.Invocation#toString unless rpcRequest is guaranteed to be not null. Still puzzled why the test case did not fail on trunk.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for the report Eric Badger.

          John Zhuge, would you be able to look into this? Thank you.

          Show
          xiaochen Xiao Chen added a comment - Thanks for the report Eric Badger . John Zhuge , would you be able to look into this? Thank you.
          Hide
          ebadger Eric Badger added a comment -

          John Zhuge, Xiao Chen, this commit has broken the following tests in branch-2.8 and branch-2:
          TestClientProtocolWithDelegationToken.testDelegationTokenRpc
          TestClientToAMTokens.testClientToAMTokens
          TestClientToAMTokens.testClientTokenRace

          Show
          ebadger Eric Badger added a comment - John Zhuge , Xiao Chen , this commit has broken the following tests in branch-2.8 and branch-2: TestClientProtocolWithDelegationToken.testDelegationTokenRpc TestClientToAMTokens.testClientToAMTokens TestClientToAMTokens.testClientTokenRace
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11843 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11843/)
          HDFS-11861. ipc.Client.Connection#sendRpcRequest should log request (jzhuge: rev 5672ae7b37ce75086a1cb5bb9a388288fc913eb7)

          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11843 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11843/ ) HDFS-11861 . ipc.Client.Connection#sendRpcRequest should log request (jzhuge: rev 5672ae7b37ce75086a1cb5bb9a388288fc913eb7) (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          jzhuge John Zhuge added a comment -

          Committed to trunk, branch-2, and branch-2.8.

          Thanks Xiao Chen for the review.

          Show
          jzhuge John Zhuge added a comment - Committed to trunk, branch-2, and branch-2.8. Thanks Xiao Chen for the review.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 13m 35s trunk passed
          +1 compile 14m 32s trunk passed
          +1 checkstyle 0m 36s trunk passed
          +1 mvnsite 1m 8s trunk passed
          -1 findbugs 1m 27s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
          +1 javadoc 0m 50s trunk passed
          +1 mvninstall 0m 41s the patch passed
          +1 compile 11m 49s the patch passed
          +1 javac 11m 49s the patch passed
          +1 checkstyle 0m 37s the patch passed
          +1 mvnsite 1m 8s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 48s the patch passed
          +1 javadoc 0m 53s the patch passed
          +1 unit 8m 54s hadoop-common in the patch passed.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          59m 54s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue HDFS-11861
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869156/HDFS-11861.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 27676b2fb0b0 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 536f057
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19805/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19805/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19805/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 13m 35s trunk passed +1 compile 14m 32s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 1m 8s trunk passed -1 findbugs 1m 27s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 0m 50s trunk passed +1 mvninstall 0m 41s the patch passed +1 compile 11m 49s the patch passed +1 javac 11m 49s the patch passed +1 checkstyle 0m 37s the patch passed +1 mvnsite 1m 8s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 53s the patch passed +1 unit 8m 54s hadoop-common in the patch passed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 59m 54s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11861 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869156/HDFS-11861.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 27676b2fb0b0 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 536f057 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19805/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19805/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19805/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John Zhuge, +1 pending jenkins. (The 5/21 one expired.)

          Show
          xiaochen Xiao Chen added a comment - Thanks John Zhuge , +1 pending jenkins. (The 5/21 one expired.)
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 13m 30s trunk passed
          +1 compile 17m 34s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 1m 5s trunk passed
          +1 mvneclipse 0m 20s trunk passed
          -1 findbugs 1m 21s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
          +1 javadoc 0m 50s trunk passed
          +1 mvninstall 0m 39s the patch passed
          +1 compile 12m 14s the patch passed
          +1 javac 12m 14s the patch passed
          +1 checkstyle 0m 36s the patch passed
          +1 mvnsite 1m 1s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 30s the patch passed
          +1 javadoc 0m 49s the patch passed
          +1 unit 7m 34s hadoop-common in the patch passed.
          +1 asflicense 0m 35s The patch does not generate ASF License warnings.
          62m 1s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue HDFS-11861
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869156/HDFS-11861.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c8038d34a6aa 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / e135c0b
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19528/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19528/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19528/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 13m 30s trunk passed +1 compile 17m 34s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 5s trunk passed +1 mvneclipse 0m 20s trunk passed -1 findbugs 1m 21s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. +1 javadoc 0m 50s trunk passed +1 mvninstall 0m 39s the patch passed +1 compile 12m 14s the patch passed +1 javac 12m 14s the patch passed +1 checkstyle 0m 36s the patch passed +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 30s the patch passed +1 javadoc 0m 49s the patch passed +1 unit 7m 34s hadoop-common in the patch passed. +1 asflicense 0m 35s The patch does not generate ASF License warnings. 62m 1s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HDFS-11861 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869156/HDFS-11861.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c8038d34a6aa 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e135c0b Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19528/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19528/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19528/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 001

          • Add request name to the debug trace in sendRpcRequest

          Sample debug trace:

          2017-05-21 00:01:22,436 [IPC Parameter Sending Thread #0] DEBUG ipc.Client (Client.java:run(1112))
           - IPC Client (852289170) connection to localhost/127.0.0.1:65012 from jzhuge 
           sending #1 org.apache.hadoop.hdfs.server.protocol.DatanodeProtocol.registerDatanode
          
          Show
          jzhuge John Zhuge added a comment - Patch 001 Add request name to the debug trace in sendRpcRequest Sample debug trace: 2017-05-21 00:01:22,436 [IPC Parameter Sending Thread #0] DEBUG ipc.Client (Client.java:run(1112)) - IPC Client (852289170) connection to localhost/127.0.0.1:65012 from jzhuge sending #1 org.apache.hadoop.hdfs.server.protocol.DatanodeProtocol.registerDatanode

            People

            • Assignee:
              jzhuge John Zhuge
              Reporter:
              jzhuge John Zhuge
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development