PD may remove wrong pending peer if the peer count exceeds the limit

[andylokandy]

Bug Report

What did you do?

The steps to reproduce:

1. Assume we have 3 stores: Store1 Store2 Store3. Region1 is a two-replica region and has 1 voter Peer1 and 1 learner Peer2:

Store1	Store2	Store3
Peer1 (leader)	Peer2 (learner)

2. PD decides to move the learner to Store3, therefore, PD adds a new learner Peer3 to Store3:

Store1	Store2	Store3
Peer1 (leader)	Peer2 (learner)	Peer3 (learner, no data, pending)

3. Store2 suffers from network jitter, which makes Peer2 fails to catch up with the leader:

Store1	Store2	Store3
Peer1 (leader)	Peer2 (learner, pending)	Peer3 (learner, no data, pending)

4. Store3's network becomes unstable and thus unable to get a snapshot from the leader. PD cancels the add peer operation on timeout. And then 3 peers exist for Region1, and PD is going to delete one of the peers:

Store1	Store2	Store3
Peer1 (leader)		Peer3 (learner, no data, pending)

5. Store1 goes down, then no data is left for Region1 in the TiKV cluster:

Store1	Store2	Store3
		Peer3 (learner, no data, pending)

What did you expect to see?

PD should not delete extra peers if the number of normal peers does not exceed the limit.

What version of PD are you using (pd-server -V)?

master

[zz-jason]

will this bug exist if there are at least 3 voters?

[zz-jason]

PD should not delete extra peers if the number of normal peers does not exceed the limit.

Sorry I didn't get the point, could you elaborate more on:

1. what are the "extra peers"?
2. what are "normal peers"?
3. what is the "limit"?

[andylokandy]

@zz-jason

will this bug exist if there are at least 3 voters?

There will be.

what are the "extra peers"? what is the "limit"?

If a region is desired to have n replicas, and we now have m replicas in fact (m > n), then there are (m - n) extra peers.

what are "normal peers"?

It means the peer is not pending.

PD should not delete extra peers if the number of normal peers does not exceed the limit.

in other words, PD should not try to keep the number of peers to the desired peer count by removing some of the peers when the total number of non-pending peers does not exceed the desired count.

[tiancaiamao]

The severity of this bug seems to be critical.

[andylokandy]

/cc @nolouch @disksing PTAL

[nolouch]

There is indeed one more replica of the learner, and it is reasonable to delete one learner. what do you expect here is to delete a replica with fewer data instead of just deleting one at random? But PD does not know the synchronization process of the replica.

[nolouch]

PD should not try to keep the number of peers to the desired peer count by removing some of the peers when the total number of non-pending peers does not exceed the desired count.

got it. it's reasonable.

[disksing]

IMO, this example is beyond the scope of our design. We should use multiple voters instead of some learners to ensure data consistency.

From the perspective of consistency model, non-pending learner means that the data is likely to be up-to-date, and pending learner means that the data is likely to be not up-to-date -- both are essentially inconsistent.

The reason for the data loss in this example is that there is only one voter, and that voter is down. Whether the PD's strategy is to not delete the extra learner, or to only delete the pending learner, or to only delete the learner with more logs, it will not change the occurrence of data loss.

[nolouch]

Is this question mainly considered in the recovery scenario? It should be an enhancement.

[andylokandy]

@disksing the problem is that pd can not distinguish between a pending peer that has no data and a pending peer that has no recent data.