[
https://issues.apache.org/jira/browse/HDFS-7189?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14278425#comment-14278425
]
Yi Liu commented on HDFS-7189:
------------------------------
Thanks Colin for the patch. Overall looks very good, only following comments:
*1).* Besides {{DFSClient}}, I see the patch passes the {{traceSampler}} to
some other classes, for example _BlockStorageLocationUtil.java_. The reason is
{{queryDatanodesForHdfsBlocksMetadata}} is invoked by
{{DFSClient#getBlockStorageLocations}}, but it uses a thread pool to call
{{VolumeBlockLocationCallable}}, so it's in different thread context. If we
don't pass {{traceSampler}}, then we will get {{NullScope}} for {{startSpan}}.
But I think the span should be in the same trace tree and as a child span
instead of start a new trace. So I think we should pass the trace info (trace
id parent span id) to continue span.
For {{CacheDirectiveIterator}}, {{CachePoolIterator}} and
{{EncryptionZoneIterato}}, it indeed can be used by user in new places of
different threads, so it's OK we pass {{traceSampler}} and can start new trace
if it's indeed in different thread.
*2).*
Some DFSClient operations don't have trace spans, should we add them?
*3).*
{code}
private static final byte[] DST = "src".getBytes(Charset.forName("UTF-8"));
{code}
Typo? It should be {{"dst"}}
*4).*
In {{getSrcDstTraceScope}}, we should also check {{dst != null}}
> Add trace spans for DFSClient metadata operations
> -------------------------------------------------
>
> Key: HDFS-7189
> URL: https://issues.apache.org/jira/browse/HDFS-7189
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Affects Versions: 2.7.0
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Attachments: HDFS-7189.001.patch, HDFS-7189.003.patch,
> HDFS-7189.004.patch, HDFS-7189.005.patch, HDFS-7189.006.patch
>
>
> We should add trace spans for DFSClient metadata operations. For example,
> {{DFSClient#rename}} should have a trace span, etc. etc.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
