The Problem about peerMsgHandler.destroyPeer

[Panlisong]

In kv/raftstore/peer_msg_handler.go:385

if  isInitialized && meta.regionRanges.Delete(&regionItem{region: d.Region()}) == nil {

consider the situation bellow:

1. cluster commit the add confChange, p0 is created by store s0, current storeMeta in s0 is: regions: map[s0 : <...>] regionRanges: <region_id: s0, region_epoch: <>>
2. peer p0 was added into region but not receive snapshot(i.e. not initialized), so the storeMeta in s0 remains constant
3. cluster commit the remove confChange while the peer p0 have not receive the remove confChange entry
4. cluster remove p0 in router, so p0 will never receive message from leader
5. next time cluster commit add p1 in s0, since p1>p0(peer_id), when in peerMsgHandler.checkMessage:265, it will call peerMsgHandler.destroyPeer to destroy p0
6. at this time, since p0 is uninitialized, the if statement upon will be short circuit so the meta.regionRanges.Delete() will not execute, so the current storeMeta in s0 is: regions: map[] regionRanges: <region_id: x, region_epoch: <>>
7. after the destroy finished, peerMsgHandler will call

kv/raftstore/peer_msg_handler.go:266
d.ctx.router.sendStore(message.NewMsg(message.MsgTypeStoreRaftMessage, msg))

7. the store will try to create peer but will fail in this statement since s0 regionRanges contains: <region_id: s0, region_epoch: <>>

kv/raftstore/store_worker.go:209
	for _, region := range meta.getOverlapRegions(&metapb.Region{
		StartKey: msg.StartKey,
		EndKey:   msg.EndKey,
	}) {
		log.Debugf("msg %s is overlapped with exist region %s", msg, region)
		if util.IsFirstVoteMessage(msg.Message) {
			meta.pendingVotes = append(meta.pendingVotes, msg)
		}
		return false, nil
	}

My solution:
reverse order to avoid short circuit

kv/raftstore/peer_msg_handler.go:385
- if  isInitialized && meta.regionRanges.Delete(&regionItem{region: d.Region()}) == nil {
+ if  meta.regionRanges.Delete(&regionItem{region: d.Region()}) == nil && isInitialized {

[Connor1996]

the region info is inserted into regionRanges only after it is initialized.

[Panlisong]

thx