[
https://issues.apache.org/jira/browse/HBASE-14451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14903324#comment-14903324
]
Colin Patrick McCabe commented on HBASE-14451:
----------------------------------------------
Thanks for this, [~stack].
{{ResultBoundedCompletionService}}: it seems like {{Tracer}} should be an
argument to the constructor here, rather than pulled from
{{Tracer#curThreadTracer}}.
{code}
requestHeaderBuilder.setTraceInfo(TracingProtos.RPCTInfo.newBuilder().
setParentId(spanId.getHigh()).
setTraceId(spanId.getLow()));
{code}
No sure if it matters, but for consistency, we should probably set {{TraceId}}
to {{spanId#getHigh}}, since that is the 64 bits that is conserved between
parent and child (in single-parent scenarios). Same comment in
{{RpcClientImpl.java}}.
{code}
protected void tracedWriteRequest(Call call, int priority, TraceScope
traceScope)
throws IOException {
try {
writeRequest(call, priority, traceScope);
} finally {
if (traceScope != null) traceScope.close();
}
}
{code}
Do we need this method any more? It seems like the calls to {{writeRequest}}
are already wrapped in try...catch blocks that we could use a traceScope with.
{{RpcClientImpl.java}}: there is a lot of awkwardness here with trying to get
the current thread tracer. Shouldn't the {{RpcClientImpl}} have its own
{{Tracer}} object internally and just use that for everything? Same comment
for {{RecoverableZooKeeper}}.
{{hbase-default.xml}}: should we also document {{hbase.htrace.sampler.classes}}?
In general, {{Tracer#curThreadTracer}} is a hack. It may be helpful in some
legacy code, but in general you should pass tracers around "normally"-- i.e.
when the {{HRegionServer}} creates objects to do what it needs to do, it should
pass them its own tracer. Remember that worker threads won't have a current
tracer when they're first created. It is always safer and cleaner to pass the
{{Tracer}} object you want in explicitly than to rely on {{curThreadTracer}}.
I don't see where we're creating the Tracer for the HBase client. I only see
us creating a tracer for the RegionServer.
{code}
cmccabe@keter:~/hbase1> git grep Tracer.Builder
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java:
this.tracer = new Tracer.Builder("RegionServer").
hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java:
this.tracer = new Tracer.Builder().name("Client").
hbase-server/src/test/java/org/apache/hadoop/hbase/trace/TestHTraceHooks.java:
new Tracer.Builder().name("test").conf(new
HBaseHTraceConfiguration(conf)).build()) {
{code}
(Note that the second two grep results are unit tests, and so don't count here)
We should trace the HBaseClient as well as the region server. And probably we
need another tracer for the HBase Master?
RingBufferTruck: I thought HBase was more of a series of tubes?
> Move on to htrace-4.0.0 (from htrace-3.2.0)
> -------------------------------------------
>
> Key: HBASE-14451
> URL: https://issues.apache.org/jira/browse/HBASE-14451
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: stack
> Attachments: 14451.txt, 14451v2.txt, 14451v3.txt, 14451v4.txt,
> 14451v5.txt, 14451v6.txt, 14451v7.txt
>
>
> htrace-4.0.0 was just release with a new API. Get up on it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
