[ 
https://issues.apache.org/jira/browse/HDFS-5274?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Masatake Iwasaki updated HDFS-5274:
-----------------------------------

    Attachment: HDFS-5274-7.patch

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


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

I removed conf from constructor argument.


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


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


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

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


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


> Add Tracing to HDFS
> -------------------
>
>                 Key: HDFS-5274
>                 URL: https://issues.apache.org/jira/browse/HDFS-5274
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>          Components: datanode, namenode
>    Affects Versions: 2.1.1-beta
>            Reporter: Elliott Clark
>            Assignee: Elliott Clark
>         Attachments: HDFS-5274-0.patch, HDFS-5274-1.patch, HDFS-5274-2.patch, 
> HDFS-5274-3.patch, HDFS-5274-4.patch, HDFS-5274-5.patch, HDFS-5274-6.patch, 
> HDFS-5274-7.patch, Zipkin   Trace a06e941b0172ec73.png, Zipkin   Trace 
> d0f0d66b8a258a69.png
>
>
> 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.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to