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

Parent spans are not initialized to NullScope for every DFSPacket

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.7.4
    • Component/s: tracing
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      The issue was found while working with PHOENIX-3752.

      Each packet received by the run() method of DataStreamer class, uses the parents field of the DFSPacket to create a new dataStreamer span, which in turn creates a writeTo span as its child span. The parents field is initialized when the packet is added to the dataQueue and the value is initialized from the ThreadLocal. This is how HTrace handles spans.
      A TraceScope is created and initialized to NullScope before the loop which runs till the point when the stream is closed.

      Consider the following scenario, when the dataQueue contains multiple packets, only the first of which has a tracing enabled. The scope is initialized to the dataStreamer scope and a writeTo span is created as its child, which gets closed once the packet is sent out to a remote datanode. Before writeTo span is started, the dataStreamer scope is detached. So calling the close method on it doesn't do anything at the end of loop.

      The second iteration will be using the stale value of the scope variable with a DFSPacket on which tracing is not enabled. This results in generation of an orphan writeTo spans which are being delivered to the SpanReceiver as registered in the TraceFramework. This may result in unlimited number of spans being generated and sent out to receiver.

      1. HDFS-11583-branch-2.7.001.patch
        2 kB
        Masatake Iwasaki
      2. HDFS-11583-branch-2.7.002.patch
        2 kB
        Masatake Iwasaki
      3. HDFS-11583-branch-2.7.003.patch
        2 kB
        Masatake Iwasaki

        Activity

        Hide
        karanmehta93 Karan Mehta added a comment -

        Since a single DFSQueue of a DataStreamer thread can encounter packets that may or may not have tracing enabled, a simple fix would be to initialize the tracing scope to NullScope.INSTANCE for every loop iteration. The scope can start a new dataStreamer scope when it finds the parents field of DFSPacket to be not empty.

        Show
        karanmehta93 Karan Mehta added a comment - Since a single DFSQueue of a DataStreamer thread can encounter packets that may or may not have tracing enabled, a simple fix would be to initialize the tracing scope to NullScope.INSTANCE for every loop iteration. The scope can start a new dataStreamer scope when it finds the parents field of DFSPacket to be not empty.
        Hide
        iwasakims Masatake Iwasaki added a comment -

        Karan Mehta , the test code of PHOENIX-3752 uses API of htrace-3.1.0-incubating.

        Since API of htrace-4 is imcompatible with htrace-3, you can not do end-to-end tracing through hbase-1 and hdfs (of hadoop-2.7). hadoop-2.7 depends on htrace-4 while hbase-1 depend on htrace-3. Starting tracing spans of htrace-3 does not affect hdfs of hadoop-2.7.

        If you could not get expected tracing spans in the test of PHOENIX-3752, the cause should not be in hdfs.

        Show
        iwasakims Masatake Iwasaki added a comment - Karan Mehta , the test code of PHOENIX-3752 uses API of htrace-3.1.0-incubating. Since API of htrace-4 is imcompatible with htrace-3, you can not do end-to-end tracing through hbase-1 and hdfs (of hadoop-2.7). hadoop-2.7 depends on htrace-4 while hbase-1 depend on htrace-3. Starting tracing spans of htrace-3 does not affect hdfs of hadoop-2.7. If you could not get expected tracing spans in the test of PHOENIX-3752 , the cause should not be in hdfs.
        Hide
        iwasakims Masatake Iwasaki added a comment -

        hadoop-2.7 depends on htrace-4 while hbase-1 depend on htrace-3.

        Sorry, that was wrong. I saw different version of code. hadoop-2.7 uses htrace-3.

        Show
        iwasakims Masatake Iwasaki added a comment - hadoop-2.7 depends on htrace-4 while hbase-1 depend on htrace-3. Sorry, that was wrong. I saw different version of code. hadoop-2.7 uses htrace-3.
        Hide
        iwasakims Masatake Iwasaki added a comment -

        I attached a patch adding test to reproduce the issue reported by Karan Mehta and the fix. branch-2 and later seems not to have same problem. The code for tracing is refactored while migrating to htrace-4.

        Show
        iwasakims Masatake Iwasaki added a comment - I attached a patch adding test to reproduce the issue reported by Karan Mehta and the fix. branch-2 and later seems not to have same problem. The code for tracing is refactored while migrating to htrace-4.
        Hide
        iwasakims Masatake Iwasaki added a comment -

        I attached 002 to avoid intermittent test failure. The test should check only relevant "writeTo" spans since thread running NameNode may send spans after client is closed.

        Show
        iwasakims Masatake Iwasaki added a comment - I attached 002 to avoid intermittent test failure. The test should check only relevant "writeTo" spans since thread running NameNode may send spans after client is closed.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 12m 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 6m 10s branch-2.7 passed
        +1 compile 0m 58s branch-2.7 passed with JDK v1.8.0_121
        +1 compile 0m 59s branch-2.7 passed with JDK v1.7.0_121
        +1 checkstyle 0m 25s branch-2.7 passed
        +1 mvnsite 0m 57s branch-2.7 passed
        +1 mvneclipse 0m 14s branch-2.7 passed
        +1 findbugs 2m 53s branch-2.7 passed
        +1 javadoc 0m 57s branch-2.7 passed with JDK v1.8.0_121
        +1 javadoc 1m 45s branch-2.7 passed with JDK v1.7.0_121
        +1 mvninstall 0m 51s the patch passed
        +1 compile 0m 55s the patch passed with JDK v1.8.0_121
        +1 javac 0m 55s the patch passed
        +1 compile 0m 57s the patch passed with JDK v1.7.0_121
        +1 javac 0m 57s the patch passed
        -0 checkstyle 0m 21s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119)
        +1 mvnsite 0m 54s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        -1 whitespace 0m 0s The patch has 1298 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        -1 whitespace 0m 34s The patch 70 line(s) with tabs.
        +1 findbugs 3m 5s the patch passed
        +1 javadoc 0m 57s the patch passed with JDK v1.8.0_121
        +1 javadoc 1m 43s the patch passed with JDK v1.7.0_121
        -1 unit 44m 52s hadoop-hdfs in the patch failed with JDK v1.7.0_121.
        -1 asflicense 0m 17s The patch generated 3 ASF License warnings.
        133m 14s



        Reason Tests
        JDK v1.8.0_121 Failed junit tests hadoop.hdfs.web.TestHttpsFileSystem
          hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
        JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot
          hadoop.hdfs.web.TestWebHDFS
          hadoop.hdfs.web.TestHttpsFileSystem
          hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:c420dfe
        JIRA Issue HDFS-11583
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861409/HDFS-11583-branch-2.7.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux f9d3fd4c66a6 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 branch-2.7 / 79f7bfb
        Default Java 1.7.0_121
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/whitespace-eol.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/whitespace-tabs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt
        JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18929/testReport/
        asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18929/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 12m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 10s branch-2.7 passed +1 compile 0m 58s branch-2.7 passed with JDK v1.8.0_121 +1 compile 0m 59s branch-2.7 passed with JDK v1.7.0_121 +1 checkstyle 0m 25s branch-2.7 passed +1 mvnsite 0m 57s branch-2.7 passed +1 mvneclipse 0m 14s branch-2.7 passed +1 findbugs 2m 53s branch-2.7 passed +1 javadoc 0m 57s branch-2.7 passed with JDK v1.8.0_121 +1 javadoc 1m 45s branch-2.7 passed with JDK v1.7.0_121 +1 mvninstall 0m 51s the patch passed +1 compile 0m 55s the patch passed with JDK v1.8.0_121 +1 javac 0m 55s the patch passed +1 compile 0m 57s the patch passed with JDK v1.7.0_121 +1 javac 0m 57s the patch passed -0 checkstyle 0m 21s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119) +1 mvnsite 0m 54s the patch passed +1 mvneclipse 0m 12s the patch passed -1 whitespace 0m 0s The patch has 1298 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply -1 whitespace 0m 34s The patch 70 line(s) with tabs. +1 findbugs 3m 5s the patch passed +1 javadoc 0m 57s the patch passed with JDK v1.8.0_121 +1 javadoc 1m 43s the patch passed with JDK v1.7.0_121 -1 unit 44m 52s hadoop-hdfs in the patch failed with JDK v1.7.0_121. -1 asflicense 0m 17s The patch generated 3 ASF License warnings. 133m 14s Reason Tests JDK v1.8.0_121 Failed junit tests hadoop.hdfs.web.TestHttpsFileSystem   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot   hadoop.hdfs.web.TestWebHDFS   hadoop.hdfs.web.TestHttpsFileSystem   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots Subsystem Report/Notes Docker Image:yetus/hadoop:c420dfe JIRA Issue HDFS-11583 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861409/HDFS-11583-branch-2.7.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f9d3fd4c66a6 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 branch-2.7 / 79f7bfb Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18929/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18929/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18929/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 18m 36s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 8m 0s branch-2.7 passed
        +1 compile 1m 0s branch-2.7 passed with JDK v1.8.0_121
        +1 compile 1m 4s branch-2.7 passed with JDK v1.7.0_121
        +1 checkstyle 0m 27s branch-2.7 passed
        +1 mvnsite 1m 1s branch-2.7 passed
        +1 mvneclipse 0m 18s branch-2.7 passed
        +1 findbugs 3m 7s branch-2.7 passed
        +1 javadoc 1m 1s branch-2.7 passed with JDK v1.8.0_121
        +1 javadoc 1m 45s branch-2.7 passed with JDK v1.7.0_121
        +1 mvninstall 0m 57s the patch passed
        +1 compile 1m 8s the patch passed with JDK v1.8.0_121
        +1 javac 1m 8s the patch passed
        +1 compile 1m 6s the patch passed with JDK v1.7.0_121
        +1 javac 1m 6s the patch passed
        -0 checkstyle 0m 27s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119)
        +1 mvnsite 1m 5s the patch passed
        +1 mvneclipse 0m 13s the patch passed
        -1 whitespace 0m 0s The patch has 1309 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        -1 whitespace 0m 35s The patch 70 line(s) with tabs.
        +1 findbugs 3m 28s the patch passed
        +1 javadoc 1m 6s the patch passed with JDK v1.8.0_121
        +1 javadoc 1m 57s the patch passed with JDK v1.7.0_121
        -1 unit 54m 12s hadoop-hdfs in the patch failed with JDK v1.7.0_121.
        -1 asflicense 0m 21s The patch generated 3 ASF License warnings.
        160m 41s



        Reason Tests
        JDK v1.8.0_121 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
          hadoop.hdfs.web.TestHttpsFileSystem
          hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
        JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.namenode.TestNamenodeCapacityReport
          hadoop.hdfs.server.datanode.TestBlockScanner
          hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
          hadoop.hdfs.TestLeaseRecovery2
          hadoop.hdfs.server.namenode.TestFileTruncate



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:c420dfe
        JIRA Issue HDFS-11583
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861418/HDFS-11583-branch-2.7.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 9bdb84e4312a 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision branch-2.7 / 79f7bfb
        Default Java 1.7.0_121
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/whitespace-eol.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/whitespace-tabs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt
        JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18931/testReport/
        asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18931/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 18m 36s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 0s branch-2.7 passed +1 compile 1m 0s branch-2.7 passed with JDK v1.8.0_121 +1 compile 1m 4s branch-2.7 passed with JDK v1.7.0_121 +1 checkstyle 0m 27s branch-2.7 passed +1 mvnsite 1m 1s branch-2.7 passed +1 mvneclipse 0m 18s branch-2.7 passed +1 findbugs 3m 7s branch-2.7 passed +1 javadoc 1m 1s branch-2.7 passed with JDK v1.8.0_121 +1 javadoc 1m 45s branch-2.7 passed with JDK v1.7.0_121 +1 mvninstall 0m 57s the patch passed +1 compile 1m 8s the patch passed with JDK v1.8.0_121 +1 javac 1m 8s the patch passed +1 compile 1m 6s the patch passed with JDK v1.7.0_121 +1 javac 1m 6s the patch passed -0 checkstyle 0m 27s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 13s the patch passed -1 whitespace 0m 0s The patch has 1309 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply -1 whitespace 0m 35s The patch 70 line(s) with tabs. +1 findbugs 3m 28s the patch passed +1 javadoc 1m 6s the patch passed with JDK v1.8.0_121 +1 javadoc 1m 57s the patch passed with JDK v1.7.0_121 -1 unit 54m 12s hadoop-hdfs in the patch failed with JDK v1.7.0_121. -1 asflicense 0m 21s The patch generated 3 ASF License warnings. 160m 41s Reason Tests JDK v1.8.0_121 Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots   hadoop.hdfs.web.TestHttpsFileSystem   hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.namenode.TestNamenodeCapacityReport   hadoop.hdfs.server.datanode.TestBlockScanner   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots   hadoop.hdfs.TestLeaseRecovery2   hadoop.hdfs.server.namenode.TestFileTruncate Subsystem Report/Notes Docker Image:yetus/hadoop:c420dfe JIRA Issue HDFS-11583 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861418/HDFS-11583-branch-2.7.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9bdb84e4312a 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.7 / 79f7bfb Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18931/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18931/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18931/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        karanmehta93 Karan Mehta added a comment - - edited

        Just for FYI,
        Current Phoenix uses HBase-1.2.3 and Hadoop - 2.7.1, which all depend on HTrace-3.1
        This fact suggests for another possible change. We also need to add traceId as one the class variables along with the parents for each DFSPacket we receive with tracing enabled. Hardcoding it to 0 doesn't seem appropriate and might as well create confusions on the Tracer output. Different traces might be perceived as a single scope since their traceId is same. Any thoughts on this? Should we create a separate JIRA for this?

        HBASE-14451 targets changing the HTrace version, but seems like it has not been committed yet.

        The patch seems good. Thanks Masatake Iwasaki.
        Although it is not really an issue, should we also consider doing the same to ResponseProcessor (redefine the scope variable to NullScope.INSTANCE? This is more of a safety measure so that we don't run into troubles if new spans might be added over there in future.

        Show
        karanmehta93 Karan Mehta added a comment - - edited Just for FYI, Current Phoenix uses HBase-1.2.3 and Hadoop - 2.7.1, which all depend on HTrace-3.1 This fact suggests for another possible change. We also need to add traceId as one the class variables along with the parents for each DFSPacket we receive with tracing enabled. Hardcoding it to 0 doesn't seem appropriate and might as well create confusions on the Tracer output. Different traces might be perceived as a single scope since their traceId is same. Any thoughts on this? Should we create a separate JIRA for this? HBASE-14451 targets changing the HTrace version, but seems like it has not been committed yet. The patch seems good. Thanks Masatake Iwasaki . Although it is not really an issue, should we also consider doing the same to ResponseProcessor (redefine the scope variable to NullScope.INSTANCE ? This is more of a safety measure so that we don't run into troubles if new spans might be added over there in future.
        Hide
        iwasakims Masatake Iwasaki added a comment -

        Although it is not really an issue, should we also consider doing the same to ResponseProcessor (redefine the scope variable to NullScope.INSTANCE? This is more of a safety measure so that we don't run into troubles if new spans might be added over there in future.

        Thanks for the review comment. I updated the patch. The warnings about checkstyle, whitespace, asflincense and unit tests in QA build are not related the the patch.

        Show
        iwasakims Masatake Iwasaki added a comment - Although it is not really an issue, should we also consider doing the same to ResponseProcessor (redefine the scope variable to NullScope.INSTANCE? This is more of a safety measure so that we don't run into troubles if new spans might be added over there in future. Thanks for the review comment. I updated the patch. The warnings about checkstyle, whitespace, asflincense and unit tests in QA build are not related the the patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 31s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 6m 5s branch-2.7 passed
        +1 compile 1m 2s branch-2.7 passed with JDK v1.8.0_121
        +1 compile 1m 0s branch-2.7 passed with JDK v1.7.0_121
        +1 checkstyle 0m 24s branch-2.7 passed
        +1 mvnsite 0m 59s branch-2.7 passed
        +1 mvneclipse 0m 15s branch-2.7 passed
        +1 findbugs 2m 54s branch-2.7 passed
        +1 javadoc 0m 57s branch-2.7 passed with JDK v1.8.0_121
        +1 javadoc 1m 43s branch-2.7 passed with JDK v1.7.0_121
        +1 mvninstall 0m 51s the patch passed
        +1 compile 0m 56s the patch passed with JDK v1.8.0_121
        +1 javac 0m 56s the patch passed
        +1 compile 0m 58s the patch passed with JDK v1.7.0_121
        +1 javac 0m 58s the patch passed
        -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119)
        +1 mvnsite 0m 55s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        -1 whitespace 0m 0s The patch has 1978 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        -1 whitespace 0m 50s The patch 70 line(s) with tabs.
        +1 findbugs 3m 3s the patch passed
        +1 javadoc 0m 54s the patch passed with JDK v1.8.0_121
        +1 javadoc 1m 41s the patch passed with JDK v1.7.0_121
        -1 unit 42m 8s hadoop-hdfs in the patch failed with JDK v1.7.0_121.
        -1 asflicense 0m 19s The patch generated 3 ASF License warnings.
        117m 7s



        Reason Tests
        JDK v1.8.0_121 Failed junit tests hadoop.hdfs.web.TestHttpsFileSystem
          hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
        JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
          hadoop.hdfs.TestDFSShell
          hadoop.hdfs.web.TestHttpsFileSystem
          hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:c420dfe
        JIRA Issue HDFS-11583
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861656/HDFS-11583-branch-2.7.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a3f003bdb7de 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 branch-2.7 / 79f7bfb
        Default Java 1.7.0_121
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/whitespace-eol.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/whitespace-tabs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt
        JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18953/testReport/
        asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18953/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 31s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 5s branch-2.7 passed +1 compile 1m 2s branch-2.7 passed with JDK v1.8.0_121 +1 compile 1m 0s branch-2.7 passed with JDK v1.7.0_121 +1 checkstyle 0m 24s branch-2.7 passed +1 mvnsite 0m 59s branch-2.7 passed +1 mvneclipse 0m 15s branch-2.7 passed +1 findbugs 2m 54s branch-2.7 passed +1 javadoc 0m 57s branch-2.7 passed with JDK v1.8.0_121 +1 javadoc 1m 43s branch-2.7 passed with JDK v1.7.0_121 +1 mvninstall 0m 51s the patch passed +1 compile 0m 56s the patch passed with JDK v1.8.0_121 +1 javac 0m 56s the patch passed +1 compile 0m 58s the patch passed with JDK v1.7.0_121 +1 javac 0m 58s the patch passed -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 117 unchanged - 2 fixed = 119 total (was 119) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 12s the patch passed -1 whitespace 0m 0s The patch has 1978 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply -1 whitespace 0m 50s The patch 70 line(s) with tabs. +1 findbugs 3m 3s the patch passed +1 javadoc 0m 54s the patch passed with JDK v1.8.0_121 +1 javadoc 1m 41s the patch passed with JDK v1.7.0_121 -1 unit 42m 8s hadoop-hdfs in the patch failed with JDK v1.7.0_121. -1 asflicense 0m 19s The patch generated 3 ASF License warnings. 117m 7s Reason Tests JDK v1.8.0_121 Failed junit tests hadoop.hdfs.web.TestHttpsFileSystem   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots JDK v1.7.0_121 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes   hadoop.hdfs.TestDFSShell   hadoop.hdfs.web.TestHttpsFileSystem   hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots Subsystem Report/Notes Docker Image:yetus/hadoop:c420dfe JIRA Issue HDFS-11583 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861656/HDFS-11583-branch-2.7.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a3f003bdb7de 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 branch-2.7 / 79f7bfb Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_121.txt JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18953/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/18953/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18953/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        karanmehta93 Karan Mehta added a comment -

        I applied the patch and built my local Phoenix code with the new Hadoop JAR and the Phoenix test now passes (Ref. PHOENIX-3752). Thanks Masatake Iwasaki.

        Show
        karanmehta93 Karan Mehta added a comment - I applied the patch and built my local Phoenix code with the new Hadoop JAR and the Phoenix test now passes (Ref. PHOENIX-3752 ). Thanks Masatake Iwasaki .
        Hide
        samarthjain Samarth Jain added a comment -

        Masatake Iwasaki

        What are your thoughts on what should be the trace id here:

        if (parents.length > 0) {
                        scope = Trace.startSpan("dataStreamer", new TraceInfo(0, parents[0]));
                        // TODO: use setParents API once it's available from HTrace 3.2
        //                scope = Trace.startSpan("dataStreamer", Sampler.ALWAYS);
        //                scope.getSpan().setParents(parents);
                      }
        
        

        Generally, traceId establishes the correlation between all related spans. If there is already an existing span in place, shouldn't we be using it's traceId instead? Probably calls for a different JIRA I think.

        Show
        samarthjain Samarth Jain added a comment - Masatake Iwasaki What are your thoughts on what should be the trace id here: if (parents.length > 0) { scope = Trace.startSpan( "dataStreamer" , new TraceInfo(0, parents[0])); // TODO: use setParents API once it's available from HTrace 3.2 // scope = Trace.startSpan( "dataStreamer" , Sampler.ALWAYS); // scope.getSpan().setParents(parents); } Generally, traceId establishes the correlation between all related spans. If there is already an existing span in place, shouldn't we be using it's traceId instead? Probably calls for a different JIRA I think.
        Hide
        jamestaylor James Taylor added a comment -

        Masatake Iwasaki - thanks so much for looking at this patch. FYI, HDFS-11622 has been filed for the issue mentioned by Samarth Jain.

        Show
        jamestaylor James Taylor added a comment - Masatake Iwasaki - thanks so much for looking at this patch. FYI, HDFS-11622 has been filed for the issue mentioned by Samarth Jain .
        Hide
        iwasakims Masatake Iwasaki added a comment -

        James Taylor, I will comment on HDFS-11622. Thanks for pinging me.

        Show
        iwasakims Masatake Iwasaki added a comment - James Taylor , I will comment on HDFS-11622 . Thanks for pinging me.
        Hide
        karanmehta93 Karan Mehta added a comment -

        Masatake Iwasaki
        When will this get committed?

        Show
        karanmehta93 Karan Mehta added a comment - Masatake Iwasaki When will this get committed?
        Hide
        iwasakims Masatake Iwasaki added a comment -

        I need +1 from another committer to commit this, pinging Colin P. McCabe and stack for review.

        Show
        iwasakims Masatake Iwasaki added a comment - I need +1 from another committer to commit this, pinging Colin P. McCabe and stack for review.
        Hide
        ajisakaa Akira Ajisaka added a comment -

        +1, checking this in.

        Show
        ajisakaa Akira Ajisaka added a comment - +1, checking this in.
        Hide
        ajisakaa Akira Ajisaka added a comment -

        Committed this to branch-2.7. Thanks Masatake Iwasaki for the fix and thanks all for the discussion.

        Show
        ajisakaa Akira Ajisaka added a comment - Committed this to branch-2.7. Thanks Masatake Iwasaki for the fix and thanks all for the discussion.
        Hide
        karanmehta93 Karan Mehta added a comment -
        Show
        karanmehta93 Karan Mehta added a comment - Thank you Akira Ajisaka and Masatake Iwasaki !

          People

          • Assignee:
            iwasakims Masatake Iwasaki
            Reporter:
            karanmehta93 Karan Mehta
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development