From 0a1eda9ee94de61d58307e2ec3daac1f940f12ce Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Wed, 20 Nov 2024 17:20:02 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #8824 close tikv/pd#8823 Signed-off-by: ti-chi-bot --- pkg/syncer/client.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/pkg/syncer/client.go b/pkg/syncer/client.go index ffbd71d2f1e..14e9b25778e 100644 --- a/pkg/syncer/client.go +++ b/pkg/syncer/client.go @@ -41,6 +41,7 @@ const ( keepaliveTime = 10 * time.Second keepaliveTimeout = 3 * time.Second msgSize = 8 * units.MiB + retryInterval = time.Second ) // StopSyncWithLeader stop to sync the region with leader. @@ -150,7 +151,12 @@ func (s *RegionSyncer) StartSyncWithLeader(addr string) { } } log.Error("server failed to establish sync stream with leader", zap.String("server", s.server.Name()), zap.String("leader", s.server.GetLeader().GetName()), errs.ZapError(err)) - time.Sleep(time.Second) + select { + case <-ctx.Done(): + log.Info("stop synchronizing with leader due to context canceled") + return + case <-time.After(retryInterval): + } continue } log.Info("server starts to synchronize with leader", zap.String("server", s.server.Name()), zap.String("leader", s.server.GetLeader().GetName()), zap.Uint64("request-index", s.history.GetNextIndex())) @@ -162,7 +168,12 @@ func (s *RegionSyncer) StartSyncWithLeader(addr string) { if err = stream.CloseSend(); err != nil { log.Error("failed to terminate client stream", errs.ZapError(errs.ErrGRPCCloseSend, err)) } - time.Sleep(time.Second) + select { + case <-ctx.Done(): + log.Info("stop synchronizing with leader due to context canceled") + return + case <-time.After(retryInterval): + } break } if s.history.GetNextIndex() != resp.GetStartIndex() { @@ -206,7 +217,17 @@ func (s *RegionSyncer) StartSyncWithLeader(addr string) { log.Debug("region is stale", zap.Stringer("origin", origin.GetMeta()), errs.ZapError(err)) continue } +<<<<<<< HEAD saveKV, _, _ := regionGuide(region, origin) +======= + cctx := &core.MetaProcessContext{ + Context: ctx, + TaskRunner: ratelimit.NewSyncRunner(), + Tracer: core.NewNoopHeartbeatProcessTracer(), + // no limit for followers. + } + saveKV, _, _, _ := regionGuide(cctx, region, origin) +>>>>>>> 41ec8dced (syncer: exit watch leader immediately (#8824)) overlaps := bc.PutRegion(region) if hasBuckets { From bc978eb41b41fcf1328bc5b34f0bc3b3d557bee6 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Thu, 21 Nov 2024 16:06:47 +0800 Subject: [PATCH 2/3] fix Signed-off-by: Ryan Leung --- pkg/syncer/client.go | 10 ---------- tests/integrations/client/client_test.go | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/syncer/client.go b/pkg/syncer/client.go index 14e9b25778e..efe0e22b878 100644 --- a/pkg/syncer/client.go +++ b/pkg/syncer/client.go @@ -217,17 +217,7 @@ func (s *RegionSyncer) StartSyncWithLeader(addr string) { log.Debug("region is stale", zap.Stringer("origin", origin.GetMeta()), errs.ZapError(err)) continue } -<<<<<<< HEAD saveKV, _, _ := regionGuide(region, origin) -======= - cctx := &core.MetaProcessContext{ - Context: ctx, - TaskRunner: ratelimit.NewSyncRunner(), - Tracer: core.NewNoopHeartbeatProcessTracer(), - // no limit for followers. - } - saveKV, _, _, _ := regionGuide(cctx, region, origin) ->>>>>>> 41ec8dced (syncer: exit watch leader immediately (#8824)) overlaps := bc.PutRegion(region) if hasBuckets { diff --git a/tests/integrations/client/client_test.go b/tests/integrations/client/client_test.go index 7fadddf4532..150e6e0e08e 100644 --- a/tests/integrations/client/client_test.go +++ b/tests/integrations/client/client_test.go @@ -228,11 +228,11 @@ func TestLeaderTransferAndMoveCluster(t *testing.T) { oldServers := cluster.GetServers() oldLeaderName := cluster.WaitLeader() for i := 0; i < 3; i++ { + time.Sleep(5 * time.Second) newPD, err := cluster.Join(ctx) re.NoError(err) re.NoError(newPD.Run()) oldLeaderName = cluster.WaitLeader() - time.Sleep(5 * time.Second) } // ABCDEF->DEF From 84f307b091b4275a36e232b302b8db0c147d4155 Mon Sep 17 00:00:00 2001 From: Ryan Leung Date: Fri, 6 Dec 2024 12:12:59 +0800 Subject: [PATCH 3/3] *: add test for syncer (#8873) ref tikv/pd#8823 Signed-off-by: Ryan Leung --- pkg/mock/mockserver/mockserver.go | 88 +++++++++++++++++++++++++++++++ pkg/syncer/client_test.go | 69 ++++++------------------ 2 files changed, 103 insertions(+), 54 deletions(-) create mode 100644 pkg/mock/mockserver/mockserver.go diff --git a/pkg/mock/mockserver/mockserver.go b/pkg/mock/mockserver/mockserver.go new file mode 100644 index 00000000000..d79d79ffa03 --- /dev/null +++ b/pkg/mock/mockserver/mockserver.go @@ -0,0 +1,88 @@ +// Copyright 2024 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mockserver + +import ( + "context" + + "github.com/pingcap/kvproto/pkg/pdpb" + "github.com/tikv/pd/pkg/core" + "github.com/tikv/pd/pkg/storage" + "github.com/tikv/pd/pkg/utils/grpcutil" +) + +// MockServer is used to mock Server for test use. +type MockServer struct { + ctx context.Context + member, leader *pdpb.Member + storage storage.Storage + bc *core.BasicCluster +} + +// NewMockServer creates a new MockServer. +func NewMockServer(ctx context.Context, member, leader *pdpb.Member, storage storage.Storage, bc *core.BasicCluster) *MockServer { + return &MockServer{ + ctx: ctx, + member: member, + leader: leader, + storage: storage, + bc: bc, + } +} + +// LoopContext returns the context of the server. +func (s *MockServer) LoopContext() context.Context { + return s.ctx +} + +// ClusterID returns the cluster ID of the server. +func (*MockServer) ClusterID() uint64 { + return 1 +} + +// GetMemberInfo returns the member info of the server. +func (s *MockServer) GetMemberInfo() *pdpb.Member { + return s.member +} + +// GetLeader returns the leader of the server. +func (s *MockServer) GetLeader() *pdpb.Member { + return s.leader +} + +// GetStorage returns the storage of the server. +func (s *MockServer) GetStorage() storage.Storage { + return s.storage +} + +// Name returns the name of the server. +func (*MockServer) Name() string { + return "mock-server" +} + +// GetRegions returns the regions of the server. +func (s *MockServer) GetRegions() []*core.RegionInfo { + return s.bc.GetRegions() +} + +// GetTLSConfig returns the TLS config of the server. +func (*MockServer) GetTLSConfig() *grpcutil.TLSConfig { + return &grpcutil.TLSConfig{} +} + +// GetBasicCluster returns the basic cluster of the server. +func (s *MockServer) GetBasicCluster() *core.BasicCluster { + return s.bc +} diff --git a/pkg/syncer/client_test.go b/pkg/syncer/client_test.go index 84193ebaffe..76b7687687d 100644 --- a/pkg/syncer/client_test.go +++ b/pkg/syncer/client_test.go @@ -21,9 +21,9 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" - "github.com/pingcap/kvproto/pkg/pdpb" "github.com/stretchr/testify/require" "github.com/tikv/pd/pkg/core" + "github.com/tikv/pd/pkg/mock/mockserver" "github.com/tikv/pd/pkg/storage" "github.com/tikv/pd/pkg/utils/grpcutil" "google.golang.org/grpc/codes" @@ -37,11 +37,13 @@ func TestLoadRegion(t *testing.T) { rs, err := storage.NewRegionStorageWithLevelDBBackend(context.Background(), tempDir, nil) re.NoError(err) - server := &mockServer{ - ctx: context.Background(), - storage: storage.NewCoreStorage(storage.NewStorageWithMemoryBackend(), rs), - bc: core.NewBasicCluster(), - } + server := mockserver.NewMockServer( + context.Background(), + nil, + nil, + storage.NewCoreStorage(storage.NewStorageWithMemoryBackend(), rs), + core.NewBasicCluster(), + ) for i := 0; i < 30; i++ { rs.SaveRegion(&metapb.Region{Id: uint64(i) + 1}) } @@ -64,11 +66,13 @@ func TestErrorCode(t *testing.T) { tempDir := t.TempDir() rs, err := storage.NewRegionStorageWithLevelDBBackend(context.Background(), tempDir, nil) re.NoError(err) - server := &mockServer{ - ctx: context.Background(), - storage: storage.NewCoreStorage(storage.NewStorageWithMemoryBackend(), rs), - bc: core.NewBasicCluster(), - } + server := mockserver.NewMockServer( + context.Background(), + nil, + nil, + storage.NewCoreStorage(storage.NewStorageWithMemoryBackend(), rs), + core.NewBasicCluster(), + ) ctx, cancel := context.WithCancel(context.TODO()) rc := NewRegionSyncer(server) conn, err := grpcutil.GetClientConn(ctx, "http://127.0.0.1", nil) @@ -79,46 +83,3 @@ func TestErrorCode(t *testing.T) { re.True(ok) re.Equal(codes.Canceled, ev.Code()) } - -type mockServer struct { - ctx context.Context - member, leader *pdpb.Member - storage storage.Storage - bc *core.BasicCluster -} - -func (s *mockServer) LoopContext() context.Context { - return s.ctx -} - -func (s *mockServer) ClusterID() uint64 { - return 1 -} - -func (s *mockServer) GetMemberInfo() *pdpb.Member { - return s.member -} - -func (s *mockServer) GetLeader() *pdpb.Member { - return s.leader -} - -func (s *mockServer) GetStorage() storage.Storage { - return s.storage -} - -func (s *mockServer) Name() string { - return "mock-server" -} - -func (s *mockServer) GetRegions() []*core.RegionInfo { - return s.bc.GetRegions() -} - -func (s *mockServer) GetTLSConfig() *grpcutil.TLSConfig { - return &grpcutil.TLSConfig{} -} - -func (s *mockServer) GetBasicCluster() *core.BasicCluster { - return s.bc -}