[
https://issues.apache.org/jira/browse/HTRACE-362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15429907#comment-15429907
]
Colin P. McCabe commented on HTRACE-362:
----------------------------------------
Thanks for the update.
htrace-kudu/pom.xml: It's good that the version is now correct, we are no
longer pulling in the spurious Hadoop dependency, and we are no longer
depending on a SNAPSHOT Maven revision.
{code}
<project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache. org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
{code}
Let's break this really long line, like in the other pom.xml files.
bq. If you are happy with changes to Kudu span receiver, After KUDU new version
is released. we could remove the Cloudera repository from htrace-kudu so that
PR can be merged later.
I'm not sure I follow completely. Are you saying that the cloudera repo in the
pom.xml is required for now?
I'm glad that you got rid of the protobuf stuff and are now using Kudu's type
system.
{code}
@Override
public void receiveSpan(Span span) {
try {
KuduTable tableSpan = client.openTable(table_span);
{code}
Opening the table for each span insertion seems like it will be very slow.
Can't we just open the table once and keep it open? Or is there some reason in
the Kudu API that we can't (I haven't looked at it that carefully). I also
don't see where we are closing this table.
{code}
spanRow.addLong(column_span_trace_id, span.getSpanId().getLow());
spanRow.addLong(column_span_span_id, span.getSpanId().getHigh());
{code}
There seems to be some confusion in nomenclature here. There is no such thing
as "trace id" in HTrace. This term exists in Zipkin and possibly in XTrace,
but not in HTrace. Span ID should be stored in a field named span id. Because
this field will need an index, it seems smarter to store the 128-bit span ID in
a single field, rather than splitting it into two 64-bit fields.
{code}
if (span.getParents().length == 0) {
spanRow.addLong(column_span_parent_id_low, 0);
spanRow.addLong(column_span_parent_id_high, 0);
spanRow.addBoolean(column_span_parent, true);
} else if (span.getParents().length > 0) {
spanRow.addLong(column_span_parent_id_low,
span.getParents()[0].getLow());
spanRow.addLong(column_span_parent_id_high,
span.getParents()[0].getHigh());
spanRow.addBoolean(column_span_parent, false);
}
{code}
Hmm. I don't follow the logic here. If there are 2 parents, we set
column_span_parent to false? And if there are 0 parents its value is
undefined? Seems wrong. We should be storing all the parents, not just one,
and I don't see a reason to have column_span_parent at all.
{code}
static final String DEFAULT_KUDU_COLUMN_SPAN_TRACE_ID = "trace_id";
{code}
Again, there's no such thing as trace id in HTrace. Did you mean "Tracer id"?
bq. 6. Eventhough Timeline annotations stored is a separate Table. Each
timeline annotation is stored with it s associated Span ID as foreign key. We
can construct the original span by referring these two tables span/annotations.
I couldnt think of a schema to persisting timeline annotations to the same span
table in respective row since there can be 0 to any number of list of
annotation per each span. Please let me know if you have concerns over this.
Even with this approach we can construct the original span after persisting it
to the DB.
There should be some way of having variable-length data in the span row.
> 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, HTRACE-362.005.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)
