Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

br: handle region leader miss (#52822) #52859

Merged
merged 7 commits into from
Jun 4, 2024
Merged

br: handle region leader miss (#52822) #52859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
220b4e6
This is an automated cherry-pick of #52822
Leavrth Apr 24, 2024
0e66c9b
fix some git conflict
lance6716 Jun 3, 2024
291ffc8
fix some conflict
lance6716 Jun 3, 2024
f46b384
fix cp
lance6716 Jun 3, 2024
95b9b6b
fix CI
lance6716 Jun 3, 2024
ae67291
fix CI
Leavrth Jun 3, 2024
83201a8
fix CI
Leavrth Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions br/pkg/lightning/backend/local/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ go_library(
importpath = "github.com/pingcap/tidb/br/pkg/lightning/backend/local",
visibility = ["//visibility:public"],
deps = [
"//br/pkg/errors",
"//br/pkg/lightning/backend",
"//br/pkg/lightning/backend/kv",
"//br/pkg/lightning/checkpoints",
Expand Down
4 changes: 3 additions & 1 deletion br/pkg/lightning/backend/local/duplicate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"github.com/pingcap/kvproto/pkg/errorpb"
"github.com/pingcap/kvproto/pkg/import_sstpb"
"github.com/pingcap/kvproto/pkg/kvrpcpb"
berrors "github.com/pingcap/tidb/br/pkg/errors"
"github.com/pingcap/tidb/br/pkg/lightning/backend/kv"
"github.com/pingcap/tidb/br/pkg/lightning/common"
"github.com/pingcap/tidb/br/pkg/lightning/errormanager"
Expand Down Expand Up @@ -297,7 +298,8 @@
) (import_sstpb.ImportSST_DuplicateDetectClient, error) {
leader := region.Leader
if leader == nil {
leader = region.Region.GetPeers()[0]
return nil, errors.Annotatef(berrors.ErrPDLeaderNotFound,
"region id %d has no leader", region.Region.Id)

Check warning on line 302 in br/pkg/lightning/backend/local/duplicate.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/lightning/backend/local/duplicate.go#L301-L302 

Added lines #L301 - L302 were not covered by tests
}
importClient, err := importClientFactory.Create(ctx, leader.GetStoreId())
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion br/pkg/lightning/backend/local/local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
sst "github.com/pingcap/kvproto/pkg/import_sstpb"
"github.com/pingcap/kvproto/pkg/kvrpcpb"
"github.com/pingcap/kvproto/pkg/metapb"
berrors "github.com/pingcap/tidb/br/pkg/errors"
"github.com/pingcap/tidb/br/pkg/lightning/backend"
"github.com/pingcap/tidb/br/pkg/lightning/backend/kv"
"github.com/pingcap/tidb/br/pkg/lightning/common"
Expand Down Expand Up @@ -1165,7 +1166,8 @@
func (local *local) Ingest(ctx context.Context, metas []*sst.SSTMeta, region *split.RegionInfo) (*sst.IngestResponse, error) {
leader := region.Leader
if leader == nil {
leader = region.Region.GetPeers()[0]
return nil, errors.Annotatef(berrors.ErrPDLeaderNotFound,
"region %d has no leader", region.Region.Id)

Check warning on line 1170 in br/pkg/lightning/backend/local/local.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/lightning/backend/local/local.go#L1169-L1170 

Added lines #L1169 - L1170 were not covered by tests
}

cli, err := local.getImportClient(ctx, leader.StoreId)
Expand Down
8 changes: 8 additions & 0 deletions br/pkg/lightning/backend/local/localhelper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ func (c *testSplitClient) SplitRegion(
ConfVer: target.Region.RegionEpoch.ConfVer + 1,
},
},
Leader: &metapb.Peer{
Id: target.Leader.Id,
StoreId: target.Leader.StoreId,
},
}
c.regions[c.nextRegionID] = newRegion
c.regionsInfo.SetRegion(pdtypes.NewRegionInfo(newRegion.Region, newRegion.Leader))
Expand Down Expand Up @@ -208,6 +212,10 @@ func (c *testSplitClient) BatchSplitRegionsWithOrigin(
StartKey: startKey,
EndKey: key,
},
Leader: &metapb.Peer{
Id: target.Leader.Id,
StoreId: target.Leader.StoreId,
},
}
c.regions[c.nextRegionID] = newRegion
c.regionsInfo.SetRegion(pdtypes.NewRegionInfo(newRegion.Region, newRegion.Leader))
Expand Down
3 changes: 2 additions & 1 deletion br/pkg/restore/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,8 @@
) (*import_sstpb.IngestResponse, error) {
leader := regionInfo.Leader
if leader == nil {
leader = regionInfo.Region.GetPeers()[0]
return nil, errors.Annotatef(berrors.ErrPDLeaderNotFound,
"region id %d has no leader", regionInfo.Region.Id)

Check warning on line 829 in br/pkg/restore/import.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/restore/import.go#L828-L829 

Added lines #L828 - L829 were not covered by tests
}
reqCtx := &kvrpcpb.Context{
RegionId: regionInfo.Region.GetId(),
Expand Down
10 changes: 6 additions & 4 deletions br/pkg/restore/import_retry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ func TestEpochNotMatch(t *testing.T) {
EndKey: right.Region.EndKey,
Id: 42,
Peers: []*metapb.Peer{
{Id: 43},
{Id: 43, StoreId: 1},
},
},
Leader: &metapb.Peer{Id: 43},
Leader: &metapb.Peer{Id: 43, StoreId: 1},
}
newRegion := pdtypes.NewRegionInfo(info.Region, info.Leader)
mergeRegion := func() {
Expand Down Expand Up @@ -304,7 +304,8 @@ func TestRegionSplit(t *testing.T) {
EndKey: codec.EncodeBytes(nil, []byte("aayy")),
},
Leader: &metapb.Peer{
Id: 43,
Id: 43,
StoreId: 1,
},
},
{
Expand All @@ -314,7 +315,8 @@ func TestRegionSplit(t *testing.T) {
EndKey: target.Region.EndKey,
},
Leader: &metapb.Peer{
Id: 45,
Id: 45,
StoreId: 1,
},
},
}
Expand Down
16 changes: 16 additions & 0 deletions br/pkg/restore/split/split.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,23 @@
}

cur := regions[0]
if cur.Leader == nil {
return errors.Annotatef(berrors.ErrPDBatchScanRegion,
"region %d's leader is nil", cur.Region.Id)
}
if cur.Leader.StoreId == 0 {
return errors.Annotatef(berrors.ErrPDBatchScanRegion,
"region %d's leader's store id is 0", cur.Region.Id)
}
for _, r := range regions[1:] {
if r.Leader == nil {
return errors.Annotatef(berrors.ErrPDBatchScanRegion,
"region %d's leader is nil", r.Region.Id)
}

Check warning on line 74 in br/pkg/restore/split/split.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/restore/split/split.go#L72-L74 

Added lines #L72 - L74 were not covered by tests
if r.Leader.StoreId == 0 {
return errors.Annotatef(berrors.ErrPDBatchScanRegion,
"region %d's leader's store id is 0", r.Region.Id)
}

Check warning on line 78 in br/pkg/restore/split/split.go

View check run for this annotation

Codecov / codecov/patch

br/pkg/restore/split/split.go#L76-L78 

Added lines #L76 - L78 were not covered by tests
if !bytes.Equal(cur.Region.EndKey, r.Region.StartKey) {
return errors.Annotatef(berrors.ErrPDBatchScanRegion, "region endKey not equal to next region startKey, endKey: %s, startKey: %s",
redact.Key(cur.Region.EndKey), redact.Key(r.Region.StartKey))
Expand Down
63 changes: 62 additions & 1 deletion br/pkg/restore/split_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,8 @@ func initTestClient(isRawKv bool) *TestClient {
}
regions[i] = &split.RegionInfo{
Leader: &metapb.Peer{
Id: i,
Id: i,
StoreId: 1,
},
Region: &metapb.Region{
Id: i,
Expand Down Expand Up @@ -691,12 +692,72 @@ func TestRegionConsistency(t *testing.T) {
StartKey: codec.EncodeBytes([]byte{}, []byte("b")),
EndKey: codec.EncodeBytes([]byte{}, []byte("d")),
},
Leader: &metapb.Peer{
Id: 6,
StoreId: 1,
},
},
{
Region: &metapb.Region{
StartKey: codec.EncodeBytes([]byte{}, []byte("e")),
EndKey: codec.EncodeBytes([]byte{}, []byte("f")),
},
Leader: &metapb.Peer{
Id: 6,
StoreId: 1,
},
},
},
},
{
codec.EncodeBytes([]byte{}, []byte("c")),
codec.EncodeBytes([]byte{}, []byte("e")),
"region 6's leader is nil(.*?)",
[]*split.RegionInfo{
{
Region: &metapb.Region{
Id: 6,
StartKey: codec.EncodeBytes([]byte{}, []byte("c")),
EndKey: codec.EncodeBytes([]byte{}, []byte("d")),
RegionEpoch: nil,
},
},
{
Region: &metapb.Region{
Id: 8,
StartKey: codec.EncodeBytes([]byte{}, []byte("d")),
EndKey: codec.EncodeBytes([]byte{}, []byte("e")),
},
},
},
},
{
codec.EncodeBytes([]byte{}, []byte("c")),
codec.EncodeBytes([]byte{}, []byte("e")),
"region 6's leader's store id is 0(.*?)",
[]*split.RegionInfo{
{
Leader: &metapb.Peer{
Id: 6,
StoreId: 0,
},
Region: &metapb.Region{
Id: 6,
StartKey: codec.EncodeBytes([]byte{}, []byte("c")),
EndKey: codec.EncodeBytes([]byte{}, []byte("d")),
RegionEpoch: nil,
},
},
{
Leader: &metapb.Peer{
Id: 6,
StoreId: 0,
},
Region: &metapb.Region{
Id: 8,
StartKey: codec.EncodeBytes([]byte{}, []byte("d")),
EndKey: codec.EncodeBytes([]byte{}, []byte("e")),
},
},
},
},
Expand Down
2 changes: 2 additions & 0 deletions br/pkg/restore/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func TestPaginateScanRegion(t *testing.T) {
Id: i + 1,
Peers: peers,
},
Leader: peers[0],
}

if i != 0 {
Expand Down Expand Up @@ -218,6 +219,7 @@ func TestPaginateScanRegion(t *testing.T) {
StartKey: endKey,
EndKey: []byte{},
},
Leader: peers[0],
}
regionsMap[num] = ri
regions = append(regions, ri)
Expand Down
Loading