[
https://issues.apache.org/jira/browse/CASSANDRA-11559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16526654#comment-16526654
]
Ariel Weisberg commented on CASSANDRA-11559:
--------------------------------------------
I am stuck at the ELI5 as to what this makes better. I would be more
comfortable with this change if we narrowed the scope to allow us to fix a
specific issue using it rather then blanket removing InetAddressAndPort
everywhere and then not using the additional information to do anything. Unless
I missed something?
Substituting a default value for the UUID when one isn't known makes me very
nervous. Either this is the canonical way of identifying something or it isn't.
I think if you don't know the value it should be an Optional and should
generate an error if you try to use a value that isn't there. And when you do
that you then find out you are in trouble because it's part of equality,
comparison, and hash code and it makes sense why that is a problem. Also the
field is mutable, but not volatile!
There is some usage of
[FBUtilities.getJustBroadcastAddress()|https://github.com/apache/cassandra/compare/trunk...alourie:CASSANDRA-11559#diff-2663bb92627e44959fc5d2b229824130R226]
that looks incorrect. Comparing addresses doesn't tell you much in 4.0.
Endpoint.getByAddressOverrideDefaults() now reads from system keyspace and
loads every single host ID just to find one. I don't know if we invoke that
method from a performance sensitive location. Usually we just
serialize/deserialize them, but it has me wondering. Also questions like are
there locking issues with threads trying to read from the system keyspace for
this while reading from the system keyspace for this and whether it is
reentrant. We might want to cache that information.
I haven't completed a line by line review yet because it's a pretty big change.
[I do see dtest
failures.|https://circleci.com/gh/aweisberg/cassandra/tree/cassandra-11559-trunk].
> Enhance node representation
> ---------------------------
>
> Key: CASSANDRA-11559
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11559
> Project: Cassandra
> Issue Type: Sub-task
> Components: Distributed Metadata
> Reporter: Paulo Motta
> Assignee: Alex Lourie
> Priority: Minor
>
> We currently represent nodes as {{InetAddress}} objects on {{TokenMetadata}},
> what causes difficulties when replacing a node with the same address (see
> CASSANDRA-8523 and CASSANDRA-9244).
> Since CASSANDRA-4120 we index hosts by {{UUID}} in gossip, so I think it's
> time to move that representation to {{TokenMetadata}}.
> I propose representing nodes as {{InetAddress, UUID}} pairs on
> {{TokenMetadata}}, encapsulated in a {{VirtualNode}} interface, so it will
> backward compatible with the current representation, while still allowing us
> to enhance it in the future with additional metadata (and improved vnode
> handling) if needed.
> This change will probably affect interfaces of internal classes like
> {{TokenMetadata}} and {{AbstractReplicationStrategy}}, so I'd like to hear
> from integrators and other developers if it's possible to change these
> without major hassle or if we need to wait until 4.0.
> Besides updating {{TokenMetadata}} and {{AbstractReplicationStrategy}} (and
> subclasses), we will also need to replace all {{InetAddress}} uses with
> {{VirtualNode.getEndpoint()}} calls on {{StorageService}} and related classes
> and tests. We would probably already be able to replace some
> {{TokenMetadata.getHostId(InetAddress endpoint)}} calls with
> {{VirtualNode.getHostId()}}.
> While we will still be dealing with {{InetAddress}} on {{StorageService}} in
> this initial stage, in the future I think we should pass {{VirtualNode}}
> instances around and only translate from {{VirtualNode}} to {{InetAddress}}
> in the network layer.
> Public interfaces like {{IEndpointSnitch}} will not be affected by this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
