Down replaced cluster members

[budde]

Make sure a previous cluster member is marked as .down before replacing it

Motivation:

• Resolve Gossip re-adds nodes replaced after ungraceful shutdown #866

Modifications:

• When processing a MemberChange that replaces an existing node, mark the node as .down if it hasn't been already
• Add an integration test for Gossip re-adds nodes replaced after ungraceful shutdown #866 (confirmed failure before this change.

Result:

• Resolves Gossip re-adds nodes replaced after ungraceful shutdown #866

[budde]

Also have a down-replaced-cluster-members-0.x in my fork against the release/0.x branch

[ktoso]

Whoa that's a nasty compiler crash there... Thank you for looking into this I will have a look!

I suggest we focus on the 0.x branch with this fix as the main branch remains in flux for a while still (with the language being in flight and all). So the failures are none of our own really but compiler issues like this one...

[ktoso]

Change looks good to me! I'll reproduce and play around a bit more tomorrow -- can you submit the PR to 0.x and let's leave main be for the time being? I'll forward port the patch later then 👍

[ktoso]

Minor typo in the integration test :)

16:29:43 /code/IntegrationTests/tests_04_cluster/test_02_ungraceful_shutdown_recovery.sh: line 31: 
thrid_logs: unbound variable

[ktoso]

Hmmm the integration test is not aggressive enough perhaps since it does not reproduce the issue but I think the fix looks
right...

[budde]

Good catch, was making a few silly assumptions in the integ test. Think the typo you caught might've been contributing to why I was getting it to fail initially ;)

Updated the test. Should fail deterministically without the fix.

[ktoso]

Great, reproduced the failure and confirmed the fix -- looking good, thank you a lot @budde !

I'll cut a release for this as well :)