[
https://issues.apache.org/jira/browse/HTRACE-362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15414708#comment-15414708
]
Colin P. McCabe commented on HTRACE-362:
----------------------------------------
Thanks, [~nisala12]. The {{pom.xml}} looks much better.
{code}
public class KuduClientConfiguration {
private final String host;
private final String port;
...
{code}
Hmm. Port should be an Integer, right? (I would normally suggest a Short, but
Java doesn't have unsigned 16-bit numbers). Same comment in
{{KuduSpanReceiver.java}}.
{code}
if (this.service != null) {
this.service.shutdownNow();
this.service = null;
}
{code}
This code in the constructor can't ever be invoked, since "service" starts out
initialized to null.
{{KuduSpanReceiver}}: I can see that you are using a BlockingQueue plus threads
to implement an async Kudu client. However, there is already an official async
Kudu client. See
http://kudu.apache.org/apidocs/org/kududb/client/AsyncKuduSession.html So we
don't need any of this stuff... just use the async client.
{code}
this.column_span_trace_id =
conf.get(KuduReceiverConstants.KUDU_COLUMN_SPAN_TRACE_ID_KEY,
KuduReceiverConstants.DEFAULT_KUDU_COLUMN_SPAN_TRACE_ID);
this.column_span_start_time =
conf.get(KuduReceiverConstants.KUDU_COLUMN_SPAN_START_TIME_KEY,
KuduReceiverConstants.DEFAULT_KUDU_COLUMN_SPAN_START_TIME);
this.column_span_stop_time =
conf.get(KuduReceiverConstants.KUDU_COLUMN_SPAN_STOP_TIME_KEY,
KuduReceiverConstants.DEFAULT_KUDU_COLUMN_SPAN_STOP_TIME);
...
{code}
Do we really need to make the names of all the columns configurable? It adds a
lot of complexity, and I can't think of a scenario where this would be useful.
If we think of a case where this would be useful, we can add it later.
On the other hand, I agree with the decision to make the table name
configurable. It lets you use the same Kudu instance as a backend for multiple
HTraces.
{code}
spanRow.addString(column_span_process_id,span.getTracerId());
{code}
There's no such thing as process id for spans in htrace 4
{code}
if (span.getParents().length == 0) {
spanRow.addLong(column_span_parent_id,0);
spanRow.addBoolean(column_span_parent,false);
} else if (span.getParents().length > 0) {
spanRow.addLong(column_span_parent_id,span.getParents()[0].getLow());
spanRow.addBoolean(column_span_parent,true);
}
{code}
This isn't correct. It's only storing one-half of one parent ID. Spans in
HTrace can have multiple parent IDs, which are all 128-bit.
I don't think we should have a separate table for timeline annotations. They
should be part of the span.
> Apache KUDU Span receiver implementation for Apache HTrace
> ----------------------------------------------------------
>
> Key: HTRACE-362
> URL: https://issues.apache.org/jira/browse/HTRACE-362
> Project: HTrace
> Issue Type: Bug
> Reporter: Nisala Mendis
> Assignee: Nisala Mendis
> Attachments: HTRACE-362.001.patch, HTRACE-362.002.patch,
> HTRACE-362.003.patch, HTRACE-362.004.patch, kudu_receiver.patch,
> kudu_span_receiver_basic_design.pdf
>
>
> Implementation should be carried two components as span receiver that writes
> kudu back end and span viewer to view written span/traces.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
