[
https://issues.apache.org/jira/browse/CASSANDRA-4383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444176#comment-13444176
]
Eric Evans commented on CASSANDRA-4383:
---------------------------------------
The approach taken here continues to make me a little hesitant simply because,
I think, it introduces for the first time a need for proper ordering of STATE
transmission/reception. I don't have a clear-enough understanding of how the
underlying messaging works to know if we can firmly rely on that or not (I take
you at your word that we can), but it is significant enough to warrant a
mention, if for the interface alone.
And on that note...
bq. Patch 0003 is something we can take or leave, it just seemed to make sense
to have a way to atomically set two gossip states at once in case the gossiper
fires in between adding the first and second state, however it's ok for us to
gossip TOKENS without STATUS, since STATUS is what fires events. I'm also not
100% certain it actually adds them atomically, since EndpointState is backed by
NBHM.
What I had in mind was (at least) something like a
{{sendState\{Normal,Bootstrap,...\}(Collection<token> tokens)}} to encapsulate
those operations sensitive to ordering.
{{Gossiper.addLocalApplicationStates(...)}} still makes it too easy to do the
wrong thing.
A couple of other things:
{{StorageService.getHostId(ep)}} creates a second means of obtaining a host ID,
and it's not at all obvious that it should only be used in assigning the value
returned by the method of the _same name_ in {{TokenMetadata}}. At a minimum,
I think this should be given a new name, one that makes obvious its purpose.
However, wouldn't encapsulation be better anyway if this were a {{Gossiper}}
method?
And in addition to {{getHostId}}, {{usesHostId}} also seems better suited to
the {{Gossiper}}.
Finally, is there any reason that {{TokenSerializer.(de)serialize(...)}}
shouldn't be static? What is the instance buying us?
----
As for the whole "...using the presence of a hostID as an implicit indication
of the version...", I was unaware of CASSANDRA-4317 (e6530cc3) and (somehow
)mistakenly took that as part of this change. Sorry about that.
bq. Well, it's six one way and a half dozen the other. We can look at
NET_VERSION instead, but it was also introduced in 1.2, so it's effectively the
same thing... and you could have the bug in the opposite direction
If we look for a NET_VERSION that's not there (and should be), then the error
is a missing NET_VERSION. If NET_VERSION is >= 1.2, or exists at all if you
prefer the implicit (I don't), and there is no HOST_ID, then we have a bug in
transmitting/reception of HOST_ID.
But, mostly I meant that it doesn't read as well as the old code that clearly
did one thing when the version was < X, and another when it was >= Y.
Anyway, this ship has sailed as far as this ticket goes, so if you'd prefer
discuss it elsewhere then that's fine. There is no need to hold any of this
against this particular issue/change.
> Binary encoding of vnode tokens
> -------------------------------
>
> Key: CASSANDRA-4383
> URL: https://issues.apache.org/jira/browse/CASSANDRA-4383
> Project: Cassandra
> Issue Type: Sub-task
> Reporter: Brandon Williams
> Assignee: Brandon Williams
> Fix For: 1.2.0 beta 1
>
> Attachments:
> 0001-Add-HOST_ID-and-TOKENS-app-states-binary-serialization.txt,
> 0002-Fix-tests.txt, 0003-Add-tokens-and-status-atomically.txt
>
>
> Since after CASSANDRA-4317 we can know which version a remote node is using
> (that is, whether it is vnode-aware or not) this a good opportunity to change
> the token encoding to binary, since with a default of 256 tokens per node
> even a fixed-length 16 byte encoding per token provides a great deal of
> savings in gossip traffic over a text representation.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
