cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joel Knighton (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-10134) Always require replace_address to replace existing address
Date Fri, 22 Apr 2016 21:09:13 GMT

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

Joel Knighton edited comment on CASSANDRA-10134 at 4/22/16 9:08 PM:
--------------------------------------------------------------------

CI looks good.

I really like the route you took here - simple and separates things that have no business
being conflated. I'm not worried about the slight change in semantics of isInitialized() -
as you mentioned, there are existing methods to obtain gossip status.

There's a bit of an interesting situation, that if anything, reinforces your point that the
semantics were unclear. In {{DynamicEndpointSnitch.updateScores()}}, we checked the initialization
status of StorageService before proceeding. I was curious whether this was intended to be
coupled to gossip or StorageService initialization; history suggests that the intent was to
couple it to StorageService initialization to accommodate weird singleton initialization patterns
which I believe are no longer relevant ([CASSANDRA-1756]). That said, your patch coupling
it to gossip preserves existing behavior, so that change in semantics doesn't particularly
concern me. On the other hand, if you want to change that check to {{StorageService.isInitialized()}}
and rerun CI, that would also work with me. Your call [~beobal]. In either case, I think a
follow-up investigating whether the check is safe to remove is worthwhile.

If you opt to keep the check coupled to {{isGossipActive()}}, feel free to commit.

EDIT: I fatfingered this and set it to InProgress, consider it "Ready to Commit" if you don't
make the above change. Sorry.


was (Author: jkni):
CI looks good.

I really like the route you took here - simple and separates things that have no business
being conflated. I'm not worried about the slight change in semantics of isInitialized() -
as you mentioned, there are existing methods to obtain gossip status.

There's a bit of an interesting situation, that if anything, reinforces your point that the
semantics were unclear. In {{DynamicEndpointSnitch.updateScores()}}, we checked the initialization
status of StorageService before proceeding. I was curious whether this was intended to be
coupled to gossip or StorageService initialization; history suggests that the intent was to
couple it to StorageService initialization to accommodate weird singleton initialization patterns
which I believe are no longer relevant ([CASSANDRA-1756]). That said, your patch coupling
it to gossip preserves existing behavior, so that change in semantics doesn't particularly
concern me. On the other hand, if you want to change that check to {{StorageService.isInitialized()}}
and rerun CI, that would also work with me. Your call [~beobal]. In either case, I think a
follow-up investigating whether the check is safe to remove is worthwhile.

If you opt to keep the check coupled to {{isGossipActive()}}, feel free to commit.

> Always require replace_address to replace existing address
> ----------------------------------------------------------
>
>                 Key: CASSANDRA-10134
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10134
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Distributed Metadata
>            Reporter: Tyler Hobbs
>            Assignee: Sam Tunnicliffe
>              Labels: docs-impacting
>             Fix For: 3.x
>
>
> Normally, when a node is started from a clean state with the same address as an existing
down node, it will fail to start with an error like this:
> {noformat}
> ERROR [main] 2015-08-19 15:07:51,577 CassandraDaemon.java:554 - Exception encountered
during startup
> java.lang.RuntimeException: A node with address /127.0.0.3 already exists, cancelling
join. Use cassandra.replace_address if you want to replace this node.
> 	at org.apache.cassandra.service.StorageService.checkForEndpointCollision(StorageService.java:543)
~[main/:na]
> 	at org.apache.cassandra.service.StorageService.prepareToJoin(StorageService.java:783)
~[main/:na]
> 	at org.apache.cassandra.service.StorageService.initServer(StorageService.java:720) ~[main/:na]
> 	at org.apache.cassandra.service.StorageService.initServer(StorageService.java:611) ~[main/:na]
> 	at org.apache.cassandra.service.CassandraDaemon.setup(CassandraDaemon.java:378) [main/:na]
> 	at org.apache.cassandra.service.CassandraDaemon.activate(CassandraDaemon.java:537) [main/:na]
> 	at org.apache.cassandra.service.CassandraDaemon.main(CassandraDaemon.java:626) [main/:na]
> {noformat}
> However, if {{auto_bootstrap}} is set to false or the node is in its own seed list, it
will not throw this error and will start normally.  The new node then takes over the host
ID of the old node (even if the tokens are different), and the only message you will see is
a warning in the other nodes' logs:
> {noformat}
> logger.warn("Changing {}'s host ID from {} to {}", endpoint, storedId, hostId);
> {noformat}
> This could cause an operator to accidentally wipe out the token information for a down
node without replacing it.  To fix this, we should check for an endpoint collision even if
{{auto_bootstrap}} is false or the node is a seed.



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

Mime
View raw message