[DISCUSS] Gossip shutdown may corrupt peers making it so the cluster never converges, and a small protocol change to fix

[David Capwell <dc...@apple.com>]

Just filed https://issues.apache.org/jira/browse/CASSANDRA-18913 (Gossip NPE due to shutdown event corrupting empty statuses) which is where I saw this issue..

When we do gossip shutdown we send a message GOSSIP_SHUTDOWN which then gets handled by this method org.apache.cassandra.gms.Gossiper#markAsShutdown… there is a issue with the current implementation; the peers mutate the state for the node shutting down, which cause pending gossip events to get ignored!

Simple example of an issue here is the following

Node1 starts up and starts bootstrapping
Node1 joins the ring
Node1 disables gossip (or halts)

In this case some nodes in the cluster will see the joining of the ring, and others won’t.  Now, the ones who have seen the gossip shutdown will set the version to Integer.MAX_VALUE which will have gossip not sync any unseen states…

Why is this a problem?  Lets say you now need to host replace node1… and the seeds you are using didn’t see the join ring event… you then get the following error during the host replacement "Could not find tokens for %s to replace”

To solve this and clean things up, I would like to send the state from the node shutting down and avoid peers mutating endpoint states they don’t own; with this the cluster should eventually converge!  This would be a protocol change, so would need to make sure everyone is cool with me doing this in 5.0.

Re: [DISCUSS] Gossip shutdown may corrupt peers making it so the cluster never converges, and a small protocol change to fix

[David Capwell <dc...@apple.com>]

Brandon and I have been talking in CASSANDRA-18913 and here is the current plan; sharing for visibility

There are two bugs: 
1) restart and seeing a shutdown event before gossip has settled for you will create a partial EndpointState which leads to failed startup
2) shutdown corrupts state due to every node modifying their local copy (this may impact host replacements)

For #1 this can be fixed without changing networking, so will create a small patch for 4.x line
For #2 it does require a network change, so will add this to 5.0 with mixed mode support 

> On Oct 6, 2023, at 4:18 PM, David Capwell <dc...@apple.com> wrote:
> 
>> Won't the replacement have a newer generation?
> 
> The replacement is a different instance.  I performs a shadow round with its seeds and if they are impacted by this issue then they are missing tokens, so we fail the host replacement… you can work around this by changing the seeds to nodes that know the token.
> 
>> I don't think it is, this is just fixing a gossip bug, and we should do so in all affected versions.
> 
> Right now we sent NoPayload which is 0 bytes, but with the change we send GossipShutdown which contains the whole EndpointState… I “feel” like 4.x can not handle this but worth a test (after deserializing the message we have extra bytes… won’t we get mad about that?)… the gossip handler doesn’t look at the payload so as long as 4.x serialization can support this, then it won’t be hard to back port to 4.x
> 
> 
>> On Oct 6, 2023, at 3:57 PM, Brandon Williams <dr...@gmail.com> wrote:
>> 
>> On Fri, Oct 6, 2023 at 5:50 PM David Capwell <dc...@apple.com> wrote:
>>> Lets say you now need to host replace node1
>> 
>> Won't the replacement have a newer generation?
>> 
>>> avoid peers mutating endpoint states they don’t own
>> 
>> This sounds reasonable to me.
>> 
>>> This would be a protocol change, so would need to make sure everyone is cool with me doing this in 5.0.>
>> 
>> I don't think it is, this is just fixing a gossip bug, and we should
>> do so in all affected versions.
>> 
>> Kind Regards,
>> Brandon
> 

Re: [DISCUSS] Gossip shutdown may corrupt peers making it so the cluster never converges, and a small protocol change to fix

[David Capwell <dc...@apple.com>]

> Won't the replacement have a newer generation?

The replacement is a different instance.  I performs a shadow round with its seeds and if they are impacted by this issue then they are missing tokens, so we fail the host replacement… you can work around this by changing the seeds to nodes that know the token.

> I don't think it is, this is just fixing a gossip bug, and we should do so in all affected versions.

Right now we sent NoPayload which is 0 bytes, but with the change we send GossipShutdown which contains the whole EndpointState… I “feel” like 4.x can not handle this but worth a test (after deserializing the message we have extra bytes… won’t we get mad about that?)… the gossip handler doesn’t look at the payload so as long as 4.x serialization can support this, then it won’t be hard to back port to 4.x


> On Oct 6, 2023, at 3:57 PM, Brandon Williams <dr...@gmail.com> wrote:
> 
> On Fri, Oct 6, 2023 at 5:50 PM David Capwell <dc...@apple.com> wrote:
>> Lets say you now need to host replace node1
> 
> Won't the replacement have a newer generation?
> 
>> avoid peers mutating endpoint states they don’t own
> 
> This sounds reasonable to me.
> 
>> This would be a protocol change, so would need to make sure everyone is cool with me doing this in 5.0.>
> 
> I don't think it is, this is just fixing a gossip bug, and we should
> do so in all affected versions.
> 
> Kind Regards,
> Brandon

Re: [DISCUSS] Gossip shutdown may corrupt peers making it so the cluster never converges, and a small protocol change to fix

[Brandon Williams <dr...@gmail.com>]

On Fri, Oct 6, 2023 at 5:50 PM David Capwell <dc...@apple.com> wrote:
> Lets say you now need to host replace node1

Won't the replacement have a newer generation?

> avoid peers mutating endpoint states they don’t own

This sounds reasonable to me.

> This would be a protocol change, so would need to make sure everyone is cool with me doing this in 5.0.>

I don't think it is, this is just fixing a gossip bug, and we should
do so in all affected versions.

Kind Regards,
Brandon