avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Doug Cutting (JIRA)" <j...@apache.org>
Subject [jira] Commented: (AVRO-595) Add Dapper-Style Tracing Plugin to Avro
Date Mon, 19 Jul 2010 21:07:52 GMT

    [ https://issues.apache.org/jira/browse/AVRO-595?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890033#action_12890033
] 

Doug Cutting commented on AVRO-595:
-----------------------------------

Looks like a good start.  Here are some comments.  I hope they're not to pedantic for this
stage of the implementation.

avroTrace.avdl
 - spurious //foo comment
 - why is the result of getSpanBlock a byte array rather than an array of spans?

SpanStorage.java
 - i don't understand the need for the query handles, and they don't appear to be used for
anything yet.
 - Why expose spans as ByteBuffers rather than as an iterator or a list or something?  Is
this an optimization, so they can be efficiently re-serialized into RPCs?  I wonder if this
is premature.
 - if getAllSpans() is just for testing, does it need to be public, or would package-private
suffice?

TracePlugin.java
 - creating a new encoder or decoder each time you read/write a long is heavyweight.  you
could instead reuse a single encoder & decoder, switching the bytes they read or write.
 - payload size should be a long instead of an int
 - do we want to serve traces on a separate port, or simply on a different url?  this seems
perhaps outside the scope of the plugin, which should just record the stats, and how they're
published might be configured elsewhere?
 - how bad would a trace ID collision be?  RANDOM.nextLong() isn't ideal, but is fast and
might be good enough if collisions aren't too terrible.  if collisions are to be avoided then
it might use something like SecureRandom.nextBytes(new byte[16]).
 - just catch IOException once, rather than around each of the metadata sets.  it should never
happen anyway.
 - hostname should be computed once and stored in a field, not per event
 - why create 100-element event vectors?  shouldn't these grow as events are added?

> Add Dapper-Style Tracing Plugin to Avro
> ---------------------------------------
>
>                 Key: AVRO-595
>                 URL: https://issues.apache.org/jira/browse/AVRO-595
>             Project: Avro
>          Issue Type: Sub-task
>            Reporter: Patrick Wendell
>            Assignee: Patrick Wendell
>         Attachments: AVRO-595.patch.v1.txt
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message