[jira] [Comment Edited] (HTRACE-209) Make span ID 128 bit to avoid collisions

["stack (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HTRACE-209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14646856#comment-14646856 ] 

stack edited comment on HTRACE-209 at 7/29/15 10:37 PM:
--------------------------------------------------------

Collisions came up at the recent Distributed Tracing Workshop organized by Adrian Cole et al. See https://docs.google.com/document/d/1OhysVVtZ0MWrRM5gU2dXZwQorb5fA2jqOe0J_Z1AjI0/edit#

One of the lads who ran original zipkin versions ("It was a weekend hack project") talked of the incidence of collisions at twitter scale being on the order of every so hours though they were throwing away the data every few days. The thought was that if you kept data longer, the likelihood of collision would only rise (There are a few notes here: https://docs.google.com/document/d/1t0hBgdY8a5MX9xEVLLrqjMhFnVqOv-dglQCDqZR__Ag/edit)

Going over the suggestion that we move to a 128bit id, my comments are same as [~iwasakims]'s.

I like the bit where top 64bits are carried through but this is just convention. Anything we can do to formalize this? Spec?

On if it walks like a UUID, then... it should be a UUID, why not type 4? 122 bits are enough?

Can we ensure a 3.x span can continue over a 4.0 process -- that'd be great?

On the patch:

Who needs to do this?

   /**
    * Set the parents of this span.<p/>
    *
    * Any existing parents will be cleared by this call.
    */
-  void setParents(long[] parents);
+  void setParents(SpanId[] parents);


Seems odd that you'd set all parents on a span? Why not set parent on creation and then add parents.... but being able to set them all seems wrong.

SpanId needs a class comment on what it is about.

TraceTree should probably go away or be renamed as TraceGraph

Otherwise, reviewed c and java changes and skimmed go set... looks good to me.


was (Author: stack):
Collisions came up at the recent Distributed Tracing Workshop organized by Adrian Cole et al. See https://docs.google.com/document/d/1OhysVVtZ0MWrRM5gU2dXZwQorb5fA2jqOe0J_Z1AjI0/edit#

One of the lads who ran original zipkin versions ("It was a weekend hack project") talked of the incidence of collisions at twitter scale being on the order of every so hours though they were throwing away the data every few days. The thought was that if you kept data longer, the likelihood of collision would only rise (There are a few notes here: https://docs.google.com/document/d/1t0hBgdY8a5MX9xEVLLrqjMhFnVqOv-dglQCDqZR__Ag/edit)

Going over the suggestion that we move to a 128bit id, my comments are same as [~iwasakims]'s.

I like the bit where top 64bits are carried through but this is just convention. Anything we can do to formalize this? Spec?

On if it walks like a UUID, then... it should be a UUID, why not type 4? 22 bits are enough?

Can we ensure a 3.x span can continue over a 4.0 process -- that'd be great?

On the patch:

Who needs to do this?

   /**
    * Set the parents of this span.<p/>
    *
    * Any existing parents will be cleared by this call.
    */
-  void setParents(long[] parents);
+  void setParents(SpanId[] parents);


Seems odd that you'd set all parents on a span? Why not set parent on creation and then add parents.... but being able to set them all seems wrong.

SpanId needs a class comment on what it is about.

TraceTree should probably go away or be renamed as TraceGraph

> Make span ID 128 bit to avoid collisions
> ----------------------------------------
>
>                 Key: HTRACE-209
>                 URL: https://issues.apache.org/jira/browse/HTRACE-209
>             Project: HTrace
>          Issue Type: Sub-task
>    Affects Versions: 4.0
>            Reporter: Colin Patrick McCabe
>            Assignee: Colin Patrick McCabe
>         Attachments: HTRACE-209.001.patch, HTRACE-209.002.patch, HTRACE-209.004.patch
>
>
> Make span ID 128 bit to avoid collisions in htrace 4.x



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)