diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index 5cdb02cb23..93ad6917b7 100644 --- a/server/cluster/cluster.go +++ b/server/cluster/cluster.go @@ -1523,15 +1523,28 @@ func (c *RaftCluster) putStoreImpl(store *metapb.Store, force bool) error { return err } - // Store address can not be the same as other stores. + // Store address and peer address can not collide across stores. + // TiFlash uses peer address for Raft replication, so a non-empty peer + // address must also not collide with any other store's address (and vice versa). + // Empty peer address is common for TiKV and should not participate in conflict checks. for _, s := range c.GetStores() { // It's OK to start a new store on the same address if the old store has been removed or physically destroyed. if s.IsRemoved() || s.IsPhysicallyDestroyed() { continue } - if s.GetID() != store.GetId() && s.GetAddress() == store.GetAddress() { + if s.GetID() == store.GetId() { + continue + } + existingAddr := s.GetAddress() + existingPeerAddr := s.GetMeta().GetPeerAddress() + if store.GetAddress() == existingAddr || + (existingPeerAddr != "" && store.GetAddress() == existingPeerAddr) { return errors.Errorf("duplicated store address: %v, already registered by %v", store, s.GetMeta()) } + if store.GetPeerAddress() != "" && + (store.GetPeerAddress() == existingAddr || store.GetPeerAddress() == existingPeerAddr) { + return errors.Errorf("duplicated store peer address: %v, already registered by %v", store, s.GetMeta()) + } } opts := make([]core.StoreCreateOption, 0) diff --git a/server/cluster/cluster_test.go b/server/cluster/cluster_test.go index b8080d7a45..61e1d55416 100644 --- a/server/cluster/cluster_test.go +++ b/server/cluster/cluster_test.go @@ -533,6 +533,144 @@ func getTestDeployPath(storeID uint64) string { return fmt.Sprintf("test/store%d", storeID) } +func TestReusePeerAddress(t *testing.T) { + re := require.New(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + _, opt, err := newTestScheduleConfig() + re.NoError(err) + cluster := newTestRaftCluster(ctx, mockid.NewIDAllocator(), opt, storage.NewStorageWithMemoryBackend()) + cluster.coordinator = schedule.NewCoordinator(ctx, cluster, nil) + + // Put 4 stores with unique main addresses but the same non-empty peer address pattern per store. + for _, store := range newTestStores(4, "2.0.0") { + meta := store.GetMeta() + meta.PeerAddress = fmt.Sprintf("mock://tiflash-peer-%d:20170", store.GetID()) + re.NoError(cluster.PutMetaStore(meta)) + } + + // Empty peer addresses should not conflict with each other. + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 100, + Address: "mock://tikv-1:20160", + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(100), + })) + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 101, + Address: "mock://tikv-2:20160", + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(101), + })) + + // Same store ID updating with the same peer address should succeed. + store1 := cluster.GetStore(1).GetMeta() + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: store1.GetId(), + Address: store1.GetAddress(), + PeerAddress: store1.GetPeerAddress(), + State: metapb.StoreState_Up, + Version: store1.GetVersion(), + DeployPath: getTestDeployPath(store1.GetId()), + })) + + // Different main address with a duplicated peer address should fail when old store is up. + re.Error(cluster.PutMetaStore(&metapb.Store{ + Id: 2001, + Address: "mock://tikv-new-1:20160", + PeerAddress: cluster.GetStore(1).GetMeta().GetPeerAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(2001), + })) + + // store 2: offline — duplicated peer address should still fail. + re.NoError(cluster.RemoveStore(2, false)) + re.Error(cluster.PutMetaStore(&metapb.Store{ + Id: 2002, + Address: "mock://tikv-new-2:20160", + PeerAddress: cluster.GetStore(2).GetMeta().GetPeerAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(2002), + })) + + // store 3: offline and physically destroyed — reuse peer address should succeed. + re.NoError(cluster.RemoveStore(3, true)) + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 2003, + Address: "mock://tikv-new-3:20160", + PeerAddress: cluster.GetStore(3).GetMeta().GetPeerAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(2003), + })) + + // store 4: tombstone — reuse peer address should succeed. + re.NoError(cluster.RemoveStore(4, true)) + re.NoError(cluster.BuryStore(4, false)) + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 2004, + Address: "mock://tikv-new-4:20160", + PeerAddress: cluster.GetStore(4).GetMeta().GetPeerAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(2004), + })) + + // Two stores with different main addresses but the same non-empty peer address. + const peerAddress = "mock://tiflash-peer:20170" + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 3001, + Address: "mock://tiflash-a:3930", + PeerAddress: peerAddress, + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(3001), + })) + re.Error(cluster.PutMetaStore(&metapb.Store{ + Id: 3002, + Address: "mock://tiflash-b:3930", + PeerAddress: peerAddress, + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(3002), + })) + + // Cross collision: new peer address equals an existing store address. + re.Error(cluster.PutMetaStore(&metapb.Store{ + Id: 4001, + Address: "mock://tiflash-c:3930", + PeerAddress: cluster.GetStore(100).GetAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(4001), + })) + + // Cross collision: new address equals an existing non-empty peer address. + re.Error(cluster.PutMetaStore(&metapb.Store{ + Id: 4002, + Address: cluster.GetStore(1).GetMeta().GetPeerAddress(), + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(4002), + })) + + // Cross reuse is allowed after the conflicting store is physically destroyed. + re.NoError(cluster.RemoveStore(100, true)) + re.NoError(cluster.PutMetaStore(&metapb.Store{ + Id: 4003, + Address: "mock://tiflash-d:3930", + PeerAddress: "mock://tikv-1:20160", + State: metapb.StoreState_Up, + Version: "2.0.0", + DeployPath: getTestDeployPath(4003), + })) +} + func TestUpStore(t *testing.T) { re := require.New(t) ctx, cancel := context.WithCancel(context.Background())