startStopReplication: Handle Existing Peer Update

[alexanderbez]

Currently, when Raft#startStopReplication is executed from a config change update, it handles two cases: adding a new peer or removing an existing peer. However, if a configuration change resulted in a peer change (e.g. server address changed), Raft#startStopReplication won't catch this and heartbeats will be sent to the old address.

e.g.

1. Create a cluster {A, B, C}
2. Stop node A & wait for view change
3. Start A with new IP address
4. Observe AddStaging is handled by Raft#appendConfigurationEntry
5. Observe new configuration is persisted
6. Observe Raft#startStopReplication is called, but Raft keeps A with the old server address
7. Observe heartbeats timeout

I'm not sure if we need to cleanup any routines or replication, so I just have the followerReplication.server field updated for now.

[briankassouf]

I'm wondering if you need to call asyncNotifyCh(s.triggerCh) like we are above?

[alexanderbez]

Perhaps? What does that do? is it idempotent?

[briankassouf]

it tells raft to kick off replication to this server

raft/replication.go

Lines 156 to 158 in ff338ca

	case <-s.triggerCh:
	lastLogIdx, _ := r.getLastLog()
	shouldStop = r.replicateTo(s, lastLogIdx)

[alexanderbez]

I see. We're only change the peer address here. So it depends on if the replication layer will pick up this change or not? If we do call asyncNotifyCh, is this idempotent? Specifically, is there any harm in calling it?

[alexanderbez]

Going to merge this. During my testing of this change, replication was still occurring so I believe the asyncNotifyCh(s.triggerCh) call is unnecessary.