Details

    • Type: New Feature New Feature
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.1.1-beta
    • Fix Version/s: None
    • Component/s: datanode, namenode
    • Labels:

      Description

      Since Google's Dapper paper has shown the benefits of tracing for a large distributed system, it seems like a good time to add tracing to HDFS. HBase has added tracing using HTrace. I propose that the same can be done within HDFS.

      1. HDFS-5274-17.patch
        86 kB
        Masatake Iwasaki
      2. HDFS-5274-16.patch
        83 kB
        Masatake Iwasaki
      3. HDFS-5274-15.patch
        84 kB
        Masatake Iwasaki
      4. HDFS-5274-14.patch
        84 kB
        Masatake Iwasaki
      5. 3node_put_200mb.png
        240 kB
        Masatake Iwasaki
      6. 3node_get_200mb.png
        117 kB
        Masatake Iwasaki
      7. 3node_put_200mb.png
        238 kB
        Masatake Iwasaki
      8. HDFS-5274-13.patch
        77 kB
        Masatake Iwasaki
      9. HDFS-5274-12.patch
        76 kB
        Masatake Iwasaki
      10. HDFS-5274-11.txt
        76 kB
        stack
      11. HDFS-5274-10.patch
        75 kB
        stack
      12. HDFS-5274-9.patch
        76 kB
        Masatake Iwasaki
      13. HDFS-5274-8.patch
        76 kB
        Masatake Iwasaki
      14. ss-5274v8-get.png
        85 kB
        Masatake Iwasaki
      15. ss-5274v8-put.png
        162 kB
        Masatake Iwasaki
      16. HDFS-5274-8.patch
        76 kB
        Masatake Iwasaki
      17. HDFS-5274-7.patch
        78 kB
        Masatake Iwasaki
      18. HDFS-5274-6.patch
        97 kB
        Elliott Clark
      19. HDFS-5274-5.patch
        92 kB
        Elliott Clark
      20. Zipkin Trace d0f0d66b8a258a69.png
        211 kB
        Elliott Clark
      21. HDFS-5274-4.patch
        91 kB
        Elliott Clark
      22. HDFS-5274-3.patch
        74 kB
        Elliott Clark
      23. HDFS-5274-2.patch
        74 kB
        Elliott Clark
      24. Zipkin Trace a06e941b0172ec73.png
        167 kB
        Elliott Clark
      25. HDFS-5274-1.patch
        59 kB
        Elliott Clark
      26. HDFS-5274-0.patch
        52 kB
        Elliott Clark

        Issue Links

          Activity

          Elliott Clark created issue -
          Hide
          Elliott Clark added a comment -

          Here's an initial implementation of the tracing. Some more annotations and instrumentation could be added if needed.

          Show
          Elliott Clark added a comment - Here's an initial implementation of the tracing. Some more annotations and instrumentation could be added if needed.
          Elliott Clark made changes -
          Field Original Value New Value
          Attachment HDFS-5274-0.patch [ 12605617 ]
          Jared Winick made changes -
          Link This issue is depended upon by ACCUMULO-1197 [ ACCUMULO-1197 ]
          Hide
          Elliott Clark added a comment -

          WIP path.

          This one has testing for the read and write paths started.

          Show
          Elliott Clark added a comment - WIP path. This one has testing for the read and write paths started.
          Elliott Clark made changes -
          Attachment HDFS-5274-1.patch [ 12606010 ]
          Elliott Clark made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Affects Version/s 2.1.1-beta [ 12324809 ]
          Hide
          Elliott Clark added a comment -

          Here's an example of what I have currently. I'm still trying to balance what should be instrumented.

          Show
          Elliott Clark added a comment - Here's an example of what I have currently. I'm still trying to balance what should be instrumented.
          Elliott Clark made changes -
          Attachment Zipkin Trace a06e941b0172ec73.png [ 12606049 ]
          Hide
          Elliott Clark added a comment -

          Instrumented Sender and Receiver (Though some of those code paths are not hit as well).
          better read side instrumentation.

          Show
          Elliott Clark added a comment - Instrumented Sender and Receiver (Though some of those code paths are not hit as well). better read side instrumentation.
          Elliott Clark made changes -
          Attachment HDFS-5274-2.patch [ 12606051 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12606051/HDFS-5274-2.patch
          against trunk revision .

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

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

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.TestEditLogJournalFailures
          org.apache.hadoop.hdfs.qjournal.TestNNWithQJM

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5072//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5072//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12606051/HDFS-5274-2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.TestEditLogJournalFailures org.apache.hadoop.hdfs.qjournal.TestNNWithQJM +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5072//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5072//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Fix for tests.

          Show
          Elliott Clark added a comment - Fix for tests.
          Elliott Clark made changes -
          Attachment HDFS-5274-3.patch [ 12606168 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12606168/HDFS-5274-3.patch
          against trunk revision .

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

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

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5078//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5078//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12606168/HDFS-5274-3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5078//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5078//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Here's a lot more rigorous testing.

          Show
          Elliott Clark added a comment - Here's a lot more rigorous testing.
          Elliott Clark made changes -
          Attachment HDFS-5274-4.patch [ 12607280 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12607280/HDFS-5274-4.patch
          against trunk revision .

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

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

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5131//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12607280/HDFS-5274-4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5131//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Another example

          Show
          Elliott Clark added a comment - Another example
          Elliott Clark made changes -
          Attachment Zipkin Trace d0f0d66b8a258a69.png [ 12607289 ]
          Elliott Clark made changes -
          Attachment HDFS-5274-5.patch [ 12607290 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12607290/HDFS-5274-5.patch
          against trunk revision .

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

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

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5133//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12607290/HDFS-5274-5.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5133//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Here's a patch that adds annotations for DFSInputStream.seek

          Show
          Elliott Clark added a comment - Here's a patch that adds annotations for DFSInputStream.seek
          Elliott Clark made changes -
          Attachment HDFS-5274-6.patch [ 12607415 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12607415/HDFS-5274-6.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5143//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5143//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12607415/HDFS-5274-6.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5143//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5143//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Ping ?

          Show
          Elliott Clark added a comment - Ping ?
          Hide
          stack added a comment -

          This facility is excellent.

          A few notes on the patch.

          Long line and formatting seems off here:

          +      if (header.hasTraceInfo() && header.getTraceInfo() != null && header.getTraceInfo().hasTraceId()) {
          +        String traceDescription = rpcRequest.toString();
          +
          +        // If the incoming RPC included tracing info, always continue the trace
          +          TraceInfo parentSpan = new TraceInfo(header.getTraceInfo().getTraceId(),
          +              header.getTraceInfo().getParentId());
          +          traceSpan = Trace.startSpan(traceDescription, parentSpan).detach();
          +        }
          

          This is a javadoc fix?

          • * @param buf - SASL wrapped request of one or more RPCs
            + * @param inBuf - SASL wrapped request of one or more RPCs

          Are you breaking your pattern of putting trace string building behind a check that tracing is enabled in the below?

          + TraceScope traceSpan = Trace.startSpan("Call " + method.getName() + " to " +
          + remoteId, CLIENT_TRACE_SAMPLER);

          nit: declare and define all in the one line rather than do initialization in constructor: + receivers = new HashSet<SpanReceiver>();

          Any reason we take config on construction and in init for SpanReceiverHost?

          SpanReceiverHost is on only when trace is enabled, right? If so, say so in class comment.

          Has to be a shutdown hook? ShutdownHookManager.get().addShutdownHook ? This is fine unless we envision someone having to override it which I suppose should never happen for an optionally enabled, rare, trace function?

          HTraceConfiguration is for testing only? Should be @visiblefortesting only or a comment at least?

          nit: Do Classname.simple() here instead? + TraceScope scope = Trace.startSpan(DFSInputStream.class.getSimpleName());

          Should there be defines for a few of these? "DFSInputStream.close" seems fine... only used once DFSInputStream.read?

          Is there something off in the formatting here?

          • private static class Packet {
          • private static final long HEART_BEAT_SEQNO = -1L;
          • long seqno; // sequencenumber of buffer in block
          • final long offsetInBlock; // offset in block
          • boolean syncBlock; // this packet forces the current block to disk
          • int numChunks; // number of chunks currently in packet
          • final int maxChunks; // max chunks in packet
            + private Span traceSpan = null;
            +
            + private static class Packet {
            + private static final long HEART_BEAT_SEQNO = -1L;
            + long seqno; // sequencenumber of buffer in block
            + final long offsetInBlock; // offset in block
            + boolean syncBlock; // this packet forces the current block to disk
            + int numChunks; // number of chunks currently in packet
            + final int maxChunks; // max chunks in packet

          Or is it just git cleaning whitespace?

          Gratuitous change?

          • ClientOperationHeaderProto header =
            + ClientOperationHeaderProto.Builder builder =
            ClientOperationHeaderProto.newBuilder()
            .setBaseHeader(buildBaseHeader(blk, blockToken))
          • .setClientName(client)
          • .build();
          • return header;
            + .setClientName(client);
            + return builder.build();

          Is an annotation of 'J' only intentional?

          + if (traceSpan != null) {
          + traceSpan.addTimelineAnnotation("J");

          Put inside a isTracing check? + Trace.addTimelineAnnotation("Connecting to downstream " + mirrorNode);

          ditto: + Trace.addTimelineAnnotation("Waiting for connect ack from downstream");

          Show
          stack added a comment - This facility is excellent. A few notes on the patch. Long line and formatting seems off here: + if (header.hasTraceInfo() && header.getTraceInfo() != null && header.getTraceInfo().hasTraceId()) { + String traceDescription = rpcRequest.toString(); + + // If the incoming RPC included tracing info, always continue the trace + TraceInfo parentSpan = new TraceInfo(header.getTraceInfo().getTraceId(), + header.getTraceInfo().getParentId()); + traceSpan = Trace.startSpan(traceDescription, parentSpan).detach(); + } This is a javadoc fix? * @param buf - SASL wrapped request of one or more RPCs + * @param inBuf - SASL wrapped request of one or more RPCs Are you breaking your pattern of putting trace string building behind a check that tracing is enabled in the below? + TraceScope traceSpan = Trace.startSpan("Call " + method.getName() + " to " + + remoteId, CLIENT_TRACE_SAMPLER); nit: declare and define all in the one line rather than do initialization in constructor: + receivers = new HashSet<SpanReceiver>(); Any reason we take config on construction and in init for SpanReceiverHost? SpanReceiverHost is on only when trace is enabled, right? If so, say so in class comment. Has to be a shutdown hook? ShutdownHookManager.get().addShutdownHook ? This is fine unless we envision someone having to override it which I suppose should never happen for an optionally enabled, rare, trace function? HTraceConfiguration is for testing only? Should be @visiblefortesting only or a comment at least? nit: Do Classname.simple() here instead? + TraceScope scope = Trace.startSpan(DFSInputStream.class.getSimpleName()); Should there be defines for a few of these? "DFSInputStream.close" seems fine... only used once DFSInputStream.read? Is there something off in the formatting here? private static class Packet { private static final long HEART_BEAT_SEQNO = -1L; long seqno; // sequencenumber of buffer in block final long offsetInBlock; // offset in block boolean syncBlock; // this packet forces the current block to disk int numChunks; // number of chunks currently in packet final int maxChunks; // max chunks in packet + private Span traceSpan = null; + + private static class Packet { + private static final long HEART_BEAT_SEQNO = -1L; + long seqno; // sequencenumber of buffer in block + final long offsetInBlock; // offset in block + boolean syncBlock; // this packet forces the current block to disk + int numChunks; // number of chunks currently in packet + final int maxChunks; // max chunks in packet Or is it just git cleaning whitespace? Gratuitous change? ClientOperationHeaderProto header = + ClientOperationHeaderProto.Builder builder = ClientOperationHeaderProto.newBuilder() .setBaseHeader(buildBaseHeader(blk, blockToken)) .setClientName(client) .build(); return header; + .setClientName(client); + return builder.build(); Is an annotation of 'J' only intentional? + if (traceSpan != null) { + traceSpan.addTimelineAnnotation("J"); Put inside a isTracing check? + Trace.addTimelineAnnotation("Connecting to downstream " + mirrorNode); ditto: + Trace.addTimelineAnnotation("Waiting for connect ack from downstream");
          Hide
          Masatake Iwasaki added a comment -

          Hi Elliott Clark,
          Are you working on this issue now?
          If you do not have enough time, I would like to help fixing and rebasing this patch.

          Show
          Masatake Iwasaki added a comment - Hi Elliott Clark , Are you working on this issue now? If you do not have enough time, I would like to help fixing and rebasing this patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12607415/HDFS-5274-6.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6158//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12607415/HDFS-5274-6.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6158//console This message is automatically generated.
          Hide
          stack added a comment -

          Masatake Iwasaki I'm pretty sure Elliott would be happy if you took it. I can help (not sure if that a hinderance or a help – smile).

          Show
          stack added a comment - Masatake Iwasaki I'm pretty sure Elliott would be happy if you took it. I can help (not sure if that a hinderance or a help – smile).
          Hide
          Masatake Iwasaki added a comment -

          Thanks stack! I'm on it.

          Show
          Masatake Iwasaki added a comment - Thanks stack ! I'm on it.
          Hide
          Masatake Iwasaki added a comment -

          I am attaching the patch rebased and updated based on review comments.

          Any reason we take config on construction and in init for SpanReceiverHost?

          I removed conf from constructor argument.

          SpanReceiverHost is on only when trace is enabled, right? If so, say so in class comment.

          SpanReceiverHost is always on, though it do nothing if no SpanReceiver is configured. I added a line in class comment.

          Has to be a shutdown hook? ShutdownHookManager.get().addShutdownHook ? This is fine unless we envision someone having to override it which I suppose should never happen for an optionally enabled, rare, trace function?

          Overriding SpanReceiverHost is not necessary, though there could be someone who implement SpanReceiver. I think it is useful to wait for receivers to process all the tracing data on crash scenario.

          HTraceConfiguration is for testing only? Should be @visiblefortesting only or a comment at least?

          HTraceConfiguration is used by SpanReceiver implementation, not for testing only.

          Should there be defines for a few of these? "DFSInputStream.close" seems fine... only used once DFSInputStream.read?

          I think it is fine not to define "DFSInputStream.read" now.


          There are some fixes in addition to above such as,

          • removed timing dependency from TestTracing.
          • added guard by Trace.isTracing() around startSpan() in DFSInputStream, FsShell and WritableRpcEngine.
          • removed SpanReceiverHost from FsShell and DFSClient. I will add options or config properties to turn on tracing from shell later on another JIRA issue.
          Show
          Masatake Iwasaki added a comment - I am attaching the patch rebased and updated based on review comments. Any reason we take config on construction and in init for SpanReceiverHost? I removed conf from constructor argument. SpanReceiverHost is on only when trace is enabled, right? If so, say so in class comment. SpanReceiverHost is always on, though it do nothing if no SpanReceiver is configured. I added a line in class comment. Has to be a shutdown hook? ShutdownHookManager.get().addShutdownHook ? This is fine unless we envision someone having to override it which I suppose should never happen for an optionally enabled, rare, trace function? Overriding SpanReceiverHost is not necessary, though there could be someone who implement SpanReceiver. I think it is useful to wait for receivers to process all the tracing data on crash scenario. HTraceConfiguration is for testing only? Should be @visiblefortesting only or a comment at least? HTraceConfiguration is used by SpanReceiver implementation, not for testing only. Should there be defines for a few of these? "DFSInputStream.close" seems fine... only used once DFSInputStream.read? I think it is fine not to define "DFSInputStream.read" now. There are some fixes in addition to above such as, removed timing dependency from TestTracing. added guard by Trace.isTracing() around startSpan() in DFSInputStream, FsShell and WritableRpcEngine. removed SpanReceiverHost from FsShell and DFSClient. I will add options or config properties to turn on tracing from shell later on another JIRA issue.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-7.patch [ 12629828 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12629828/HDFS-5274-7.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestHASafeMode
          org.apache.hadoop.hdfs.server.namenode.TestCacheDirectives

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6174//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6174//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12629828/HDFS-5274-7.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestHASafeMode org.apache.hadoop.hdfs.server.namenode.TestCacheDirectives +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6174//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6174//console This message is automatically generated.
          Hide
          stack added a comment -

          My guess is that the failures are unrelated. We can rerun the patch or just wait on next iteration.

          Patch looks great to me. Have you tried it outside of the unit tests to make sure you get sensible looking spans and numbers? Perhaps I can help here?

          Fix these in next patch:

          "+ * This class do nothing If no SpanReceiver is configured ."

          "+ * Trancing information of HTrace, if exists."

          Is formatting ok here?

          + if (source != null)

          { + proto.setSource(PBHelper.convertDatanodeInfo(source)); + }

          + send(out, Op.WRITE_BLOCK, proto.build());
          + } finally

          { + if (ts != null) ts.close(); + }

          In BlockReceiver, should traceSpan be getting closed?

          Is it possible that below throws an exception?

          + scope.getSpan().addKVAnnotation(
          + "stream".getBytes(),
          + jas.getCurrentStream().toString().getBytes());

          i..e. we can hope out w/o closing the span since the try/finally only happens later.

          This is in JournalSet in a few places.

          TraceInfo and RPCTInfo seem to be same datastructure? Should we define it onetime only and share?'

          Show
          stack added a comment - My guess is that the failures are unrelated. We can rerun the patch or just wait on next iteration. Patch looks great to me. Have you tried it outside of the unit tests to make sure you get sensible looking spans and numbers? Perhaps I can help here? Fix these in next patch: "+ * This class do nothing If no SpanReceiver is configured ." "+ * Trancing information of HTrace, if exists." Is formatting ok here? + if (source != null) { + proto.setSource(PBHelper.convertDatanodeInfo(source)); + } + send(out, Op.WRITE_BLOCK, proto.build()); + } finally { + if (ts != null) ts.close(); + } In BlockReceiver, should traceSpan be getting closed? Is it possible that below throws an exception? + scope.getSpan().addKVAnnotation( + "stream".getBytes(), + jas.getCurrentStream().toString().getBytes()); i..e. we can hope out w/o closing the span since the try/finally only happens later. This is in JournalSet in a few places. TraceInfo and RPCTInfo seem to be same datastructure? Should we define it onetime only and share?'
          Hide
          Masatake Iwasaki added a comment -

          Hi stack, thanks for your review comments.

          Have you tried it outside of the unit tests to make sure you get sensible looking spans and numbers?

          I checked the trace of putting and getting a big file by Zipkin today. There seems to be too many spans concerning "DFSInputStream.read" and "DFSOutputStream.write". I will fix this in the next version of patch.

          Show
          Masatake Iwasaki added a comment - Hi stack , thanks for your review comments. Have you tried it outside of the unit tests to make sure you get sensible looking spans and numbers? I checked the trace of putting and getting a big file by Zipkin today. There seems to be too many spans concerning "DFSInputStream.read" and "DFSOutputStream.write". I will fix this in the next version of patch.
          Hide
          Suresh Srinivas added a comment -

          Is there any plan on making HTrace libraries org.cloudera.htrace.* available in Hadoop common?

          Show
          Suresh Srinivas added a comment - Is there any plan on making HTrace libraries org.cloudera.htrace.* available in Hadoop common?
          Hide
          Todd Lipcon added a comment -

          Hi Suresh. As one of the primary authors of HTrace I'd say that there are no plans to put it in Hadoop Common. Personally I think Hadoop Common as a grab-bag of all things Hadoop gets pretty messy, since it's harder to piece-meal upgrade different components. It also means that dependent apps like HBase would need to wait on new versions of Common and have even more difficulty building the same code against different versions of Hadoop.

          Happy to take contributions to HTrace via github pull request if you like, though.

          Show
          Todd Lipcon added a comment - Hi Suresh. As one of the primary authors of HTrace I'd say that there are no plans to put it in Hadoop Common. Personally I think Hadoop Common as a grab-bag of all things Hadoop gets pretty messy, since it's harder to piece-meal upgrade different components. It also means that dependent apps like HBase would need to wait on new versions of Common and have even more difficulty building the same code against different versions of Hadoop. Happy to take contributions to HTrace via github pull request if you like, though.
          Hide
          stack added a comment -

          Masatake Iwasaki Thanks. When you are done, I'll try hooking it up w/ hbase to make sure we get a trace that spans the two systems.

          Suresh Srinivas Wouldn't adding htrace to the common pom.xml make it "...available in Hadoop common"? Thanks.

          Show
          stack added a comment - Masatake Iwasaki Thanks. When you are done, I'll try hooking it up w/ hbase to make sure we get a trace that spans the two systems. Suresh Srinivas Wouldn't adding htrace to the common pom.xml make it "...available in Hadoop common"? Thanks.
          Hide
          Suresh Srinivas added a comment -

          Wouldn't adding htrace to the common pom.xml make it "...available in Hadoop common"? Thanks.

          I agree with the comment Doug Cutting had made - https://issues.apache.org/jira/browse/HADOOP-10311?focusedCommentId=13886809&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13886809. I would love to see the code in Hadoop community itself, if possible. Agreed, this is not different using google guava or other such libraries. But I am afraid this could start a trend of capabilities hosted/attributed especially to vendor companies making its way into Hadoop.

          With that said, I am -0 and would rather not see this becoming a trend.

          Show
          Suresh Srinivas added a comment - Wouldn't adding htrace to the common pom.xml make it "...available in Hadoop common"? Thanks. I agree with the comment Doug Cutting had made - https://issues.apache.org/jira/browse/HADOOP-10311?focusedCommentId=13886809&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13886809 . I would love to see the code in Hadoop community itself, if possible. Agreed, this is not different using google guava or other such libraries. But I am afraid this could start a trend of capabilities hosted/attributed especially to vendor companies making its way into Hadoop. With that said, I am -0 and would rather not see this becoming a trend.
          Hide
          stack added a comment -

          Masatake Iwasaki You know, I was thinking... Maybe it ok that there are so many spans? Tracing doesn't cost unless enabled. When debugging, you might want to see in the trace that HDFS is doing a bunch of small reads?

          Show
          stack added a comment - Masatake Iwasaki You know, I was thinking... Maybe it ok that there are so many spans? Tracing doesn't cost unless enabled. When debugging, you might want to see in the trace that HDFS is doing a bunch of small reads?
          Hide
          Masatake Iwasaki added a comment -

          I am attaching updated patch and screen shots of trace of putting and getting a 200MB file.

          Fix these in next patch:

          fixed.

          Is formatting ok here?

          fixed.

          In BlockReceiver, should traceSpan be getting closed?

          added description to span and calling close().

          Is it possible that below throws an exception?

          + scope.getSpan().addKVAnnotation(
          + "stream".getBytes(),
          + jas.getCurrentStream().toString().getBytes());

          i..e. we can hope out w/o closing the span since the try/finally only happens later.

          This is in JournalSet in a few places.

          I moved these code in try block to make sure.

          TraceInfo and RPCTInfo seem to be same datastructure? Should we define it onetime only and share?'

          I prefer keeping this as is because of simplicity and independency between datatransfer protocol and o.a.h.ipc.

          I checked the trace of putting and getting a big file by Zipkin today. There seems to be too many spans concerning "DFSInputStream.read" and "DFSOutputStream.write". I will fix this in the next version of patch.

          just removed those spans from DFSInputStream and DFSOutputStream.

          Show
          Masatake Iwasaki added a comment - I am attaching updated patch and screen shots of trace of putting and getting a 200MB file. Fix these in next patch: fixed. Is formatting ok here? fixed. In BlockReceiver, should traceSpan be getting closed? added description to span and calling close(). Is it possible that below throws an exception? + scope.getSpan().addKVAnnotation( + "stream".getBytes(), + jas.getCurrentStream().toString().getBytes()); i..e. we can hope out w/o closing the span since the try/finally only happens later. This is in JournalSet in a few places. I moved these code in try block to make sure. TraceInfo and RPCTInfo seem to be same datastructure? Should we define it onetime only and share?' I prefer keeping this as is because of simplicity and independency between datatransfer protocol and o.a.h.ipc. I checked the trace of putting and getting a big file by Zipkin today. There seems to be too many spans concerning "DFSInputStream.read" and "DFSOutputStream.write". I will fix this in the next version of patch. just removed those spans from DFSInputStream and DFSOutputStream.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-8.patch [ 12630257 ]
          Attachment ss-5274v8-put.png [ 12630258 ]
          Attachment ss-5274v8-get.png [ 12630259 ]
          Hide
          Masatake Iwasaki added a comment -

          Masatake Iwasaki You know, I was thinking... Maybe it ok that there are so many spans? Tracing doesn't cost unless enabled. When debugging, you might want to see in the trace that HDFS is doing a bunch of small reads?

          I just missed your comment on uploading v.8 patch.
          Because there were so many spans than I expected and receiver's queue was filled, I think disabling those spans is safer as a starting point.

          14/02/19 22:25:23 ERROR impl.ZipkinSpanReceiver: Error trying to append span (DFSOutputStream.write) to the queue.  Blocking Queue was full.
          
          Show
          Masatake Iwasaki added a comment - Masatake Iwasaki You know, I was thinking... Maybe it ok that there are so many spans? Tracing doesn't cost unless enabled. When debugging, you might want to see in the trace that HDFS is doing a bunch of small reads? I just missed your comment on uploading v.8 patch. Because there were so many spans than I expected and receiver's queue was filled, I think disabling those spans is safer as a starting point. 14/02/19 22:25:23 ERROR impl.ZipkinSpanReceiver: Error trying to append span (DFSOutputStream.write) to the queue. Blocking Queue was full.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12630259/ss-5274v8-get.png
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6204//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12630259/ss-5274v8-get.png against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6204//console This message is automatically generated.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-8.patch [ 12630393 ]
          Hide
          stack added a comment -

          But I am afraid this could start a trend of capabilities hosted/attributed especially to vendor companies making its way into Hadoop.

          Suresh Srinivas I just got perms on this lib. I'll go get a domain, convert the code package names, make a new release, and publish to maven central... will report back when done.

          Show
          stack added a comment - But I am afraid this could start a trend of capabilities hosted/attributed especially to vendor companies making its way into Hadoop. Suresh Srinivas I just got perms on this lib. I'll go get a domain, convert the code package names, make a new release, and publish to maven central... will report back when done.
          Hide
          Suresh Srinivas added a comment -

          Thanks stack!

          Show
          Suresh Srinivas added a comment - Thanks stack !
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12630393/HDFS-5274-8.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.web.TestWebHDFS

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6208//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6208//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12630393/HDFS-5274-8.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.web.TestWebHDFS +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6208//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6208//console This message is automatically generated.
          Hide
          stack added a comment -

          I put up a SNAPSHOT of the tracing jar w/ its groupId changed from org.cloudera.htrace to org.htrace. The version is currently 3.0-SNAPSHOT (all is the same except for the redone domain). If it works for you Masatake Iwasaki, I'll make a release (if the former owners of this jar are good w/ my manglings). Thanks boss.

          Show
          stack added a comment - I put up a SNAPSHOT of the tracing jar w/ its groupId changed from org.cloudera.htrace to org.htrace. The version is currently 3.0-SNAPSHOT (all is the same except for the redone domain). If it works for you Masatake Iwasaki , I'll make a release (if the former owners of this jar are good w/ my manglings). Thanks boss.
          Hide
          Masatake Iwasaki added a comment -

          Thanks stack! It seems not yet to be visible in repo.. I will check again a few hours later.

          Show
          Masatake Iwasaki added a comment - Thanks stack ! It seems not yet to be visible in repo.. I will check again a few hours later.
          Hide
          Masatake Iwasaki added a comment -

          Hi stack, it worked! Please make a release. I will attach a rebased patch later.

          Show
          Masatake Iwasaki added a comment - Hi stack , it worked! Please make a release. I will attach a rebased patch later.
          Hide
          stack added a comment -

          Hmm... Add this repository in the top level hadoop pom?

          + <repository>
          + <id>oss.sonatype</id>
          + <name>Sonatype OSS Repository</name>
          + <url>https://oss.sonatype.org/content/groups/public</url>
          + <snapshots>
          + <enabled>true</enabled>
          + </snapshots>
          + <releases>
          + <enabled>true</enabled>
          + </releases>
          + </repository>

          That makes it work for me. Thanks Masatake Iwasaki

          Show
          stack added a comment - Hmm... Add this repository in the top level hadoop pom? + <repository> + <id>oss.sonatype</id> + <name>Sonatype OSS Repository</name> + <url> https://oss.sonatype.org/content/groups/public </url> + <snapshots> + <enabled>true</enabled> + </snapshots> + <releases> + <enabled>true</enabled> + </releases> + </repository> That makes it work for me. Thanks Masatake Iwasaki
          Hide
          Masatake Iwasaki added a comment -

          attaching updated patch.

          • rebased to trunk,
          • changed package name from org.cloudera.htrace to org.htrace,
          • added repository to top level pom.
          Show
          Masatake Iwasaki added a comment - attaching updated patch. rebased to trunk, changed package name from org.cloudera.htrace to org.htrace, added repository to top level pom.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-9.patch [ 12631366 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12631366/HDFS-5274-9.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The test build failed in hadoop-hdfs-project/hadoop-hdfs

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6249//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6249//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12631366/HDFS-5274-9.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The test build failed in hadoop-hdfs-project/hadoop-hdfs +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6249//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6249//console This message is automatically generated.
          Hide
          stack added a comment -

          Rebase and updated the patch to refer to released htrace (which is now in maven central so no need of the sonatype repo reference).

          Show
          stack added a comment - Rebase and updated the patch to refer to released htrace (which is now in maven central so no need of the sonatype repo reference).
          stack made changes -
          Attachment HDFS-5274-10.patch [ 12632181 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12632181/HDFS-5274-10.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings.

          -1 core tests. The test build failed in hadoop-hdfs-project/hadoop-hdfs

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12632181/HDFS-5274-10.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 1 release audit warnings. -1 core tests . The test build failed in hadoop-hdfs-project/hadoop-hdfs +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6274//console This message is automatically generated.
          Hide
          stack added a comment -

          + Fix double-close in Server.java
          + Minor fix of javadoc comment
          + Fix few places where we were not testing if trace enabled.

          Show
          stack added a comment - + Fix double-close in Server.java + Minor fix of javadoc comment + Fix few places where we were not testing if trace enabled.
          stack made changes -
          Attachment HDFS-5274-11.txt [ 12632352 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12632352/HDFS-5274-11.txt
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 6 new Findbugs (version 1.3.9) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.tracing.TestTracing

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12632352/HDFS-5274-11.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 6 new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 1 release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.tracing.TestTracing +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6281//console This message is automatically generated.
          Hide
          Masatake Iwasaki added a comment -

          fixed the test if tracing in Receiver.

          Show
          Masatake Iwasaki added a comment - fixed the test if tracing in Receiver.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-12.patch [ 12632559 ]
          Hide
          Masatake Iwasaki added a comment -

          added tests for the cases tracing disabled.

          Show
          Masatake Iwasaki added a comment - added tests for the cases tracing disabled.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-13.patch [ 12632567 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12632559/HDFS-5274-12.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6297//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6297//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12632559/HDFS-5274-12.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6297//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6297//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12632567/HDFS-5274-13.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.cli.TestAclCLI

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6298//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6298//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12632567/HDFS-5274-13.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.cli.TestAclCLI +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6298//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6298//console This message is automatically generated.
          Hide
          Masatake Iwasaki added a comment -

          adding screen shots of tracing on 3node cluster: 3node_put_200mb.png and 3node_get_200mb.png

          Show
          Masatake Iwasaki added a comment - adding screen shots of tracing on 3node cluster: 3node_put_200mb.png and 3node_get_200mb.png
          Masatake Iwasaki made changes -
          Attachment 3node_put_200mb.png [ 12633484 ]
          Attachment 3node_get_200mb.png [ 12633485 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12633485/3node_get_200mb.png
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6349//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12633485/3node_get_200mb.png against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6349//console This message is automatically generated.
          Masatake Iwasaki made changes -
          Attachment 3node_put_200mb.png [ 12633965 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12633965/3node_put_200mb.png
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6373//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12633965/3node_put_200mb.png against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6373//console This message is automatically generated.
          Hide
          stack added a comment -

          Masatake Iwasaki That is a beautiful png

          Show
          stack added a comment - Masatake Iwasaki That is a beautiful png
          stack made changes -
          Link This issue is related to HBASE-6449 [ HBASE-6449 ]
          Hide
          Masatake Iwasaki added a comment -

          attaching updated patch.

          • rebased to current trunk
          • bumped the version of htrace to 3.0.3
          • added documentation of tracing

          stack I did not add htrace-zipkin to the dependency of Hadoop in order to keep hadoop-common as clean as possible. I added the documentation including the setup procedure of htrace-zipkin instead.

          Show
          Masatake Iwasaki added a comment - attaching updated patch. rebased to current trunk bumped the version of htrace to 3.0.3 added documentation of tracing stack I did not add htrace-zipkin to the dependency of Hadoop in order to keep hadoop-common as clean as possible. I added the documentation including the setup procedure of htrace-zipkin instead.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-14.patch [ 12637769 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12637769/HDFS-5274-14.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6560//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6560//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12637769/HDFS-5274-14.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6560//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6560//console This message is automatically generated.
          Hide
          Masatake Iwasaki added a comment -

          I added htrace-zipkin to dependency. All of the dependency of htrace-zipkin except libthrift is already bundled with Hadoop.

          Show
          Masatake Iwasaki added a comment - I added htrace-zipkin to dependency. All of the dependency of htrace-zipkin except libthrift is already bundled with Hadoop.
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-15.patch [ 12638024 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12638024/HDFS-5274-15.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6566//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6566//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12638024/HDFS-5274-15.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6566//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6566//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Please don't add htrace-zpikin as a dependency. That version of thrift pulled in is very old and we don't want to have classpath issues because of a new feature.

          Show
          Elliott Clark added a comment - Please don't add htrace-zpikin as a dependency. That version of thrift pulled in is very old and we don't want to have classpath issues because of a new feature.
          Hide
          Masatake Iwasaki added a comment -

          attaching updated patch.

          • rebased
          • removed htrace-zipkin from pom.xml
          Show
          Masatake Iwasaki added a comment - attaching updated patch. rebased removed htrace-zipkin from pom.xml
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-16.patch [ 12640555 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12640555/HDFS-5274-16.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.balancer.TestBalancerWithNodeGroup

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6680//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6680//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12640555/HDFS-5274-16.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.balancer.TestBalancerWithNodeGroup +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6680//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6680//console This message is automatically generated.
          Hide
          Masatake Iwasaki added a comment -

          I updated the patch

          • rebased to trunk
          • fixed the init/close logic of SpanReceiverHost
          • bumped the version of htrace to 3.0.4
          Show
          Masatake Iwasaki added a comment - I updated the patch rebased to trunk fixed the init/close logic of SpanReceiverHost bumped the version of htrace to 3.0.4
          Masatake Iwasaki made changes -
          Attachment HDFS-5274-17.patch [ 12660003 ]
          Hide
          Masatake Iwasaki added a comment -

          By the way, should I split the patch for ease of review?
          The patch could be split in two as

          1. common part such as fixes in RPC and span receivers,
          2. codes for tracing of HDFS logic.

          Once the former is merged, developers can write their own (temporary) tracing code for developing features.
          I appreciate the comments from reviewers and committers. Thanks.

          Show
          Masatake Iwasaki added a comment - By the way, should I split the patch for ease of review? The patch could be split in two as common part such as fixes in RPC and span receivers, codes for tracing of HDFS logic. Once the former is merged, developers can write their own (temporary) tracing code for developing features. I appreciate the comments from reviewers and committers. Thanks.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 4 new or modified test files.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

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

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
          org.apache.hadoop.hdfs.server.namenode.ha.TestDFSZKFailoverController

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover org.apache.hadoop.hdfs.server.namenode.ha.TestDFSZKFailoverController +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/7566//console This message is automatically generated.
          Hide
          Masatake Iwasaki added a comment -

          I am going to split the patch into 3 pieces.

          1. Adding tracing to Hadoop RPC
          2. Adding tracing to DataNode data transfer protocol
          3. Adding tracing spans to HDFS

          #2 depends on #1 and #3 depends on #2.

          Show
          Masatake Iwasaki added a comment - I am going to split the patch into 3 pieces. 1. Adding tracing to Hadoop RPC 2. Adding tracing to DataNode data transfer protocol 3. Adding tracing spans to HDFS #2 depends on #1 and #3 depends on #2.
          Hide
          Nick Dimiduk added a comment -

          thanks for sticking with this one Masatake Iwasaki!

          Show
          Nick Dimiduk added a comment - thanks for sticking with this one Masatake Iwasaki !
          Masatake Iwasaki made changes -
          Link This issue relates to HDFS-7001 [ HDFS-7001 ]
          Hide
          Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch
          Optional Tests javadoc javac unit findbugs checkstyle site
          git revision trunk / f1a152c
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10658/console

          This message was automatically generated.

          Show
          Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch Optional Tests javadoc javac unit findbugs checkstyle site git revision trunk / f1a152c Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10658/console This message was automatically generated.
          Hide
          Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch
          Optional Tests javadoc javac unit findbugs checkstyle site
          git revision trunk / f1a152c
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10669/console

          This message was automatically generated.

          Show
          Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12660003/HDFS-5274-17.patch Optional Tests javadoc javac unit findbugs checkstyle site git revision trunk / f1a152c Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10669/console This message was automatically generated.
          Allen Wittenauer made changes -
          Labels BB2015-05-TBR
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          2d 23h 19m 1 Elliott Clark 01/Oct/13 01:11

            People

            • Assignee:
              Elliott Clark
              Reporter:
              Elliott Clark
            • Votes:
              1 Vote for this issue
              Watchers:
              38 Start watching this issue

              Dates

              • Created:
                Updated:

                Development