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.

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

api: fix panic when region doesn't have a leader (#7629) #7649

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ func (c *client) ScanRegions(ctx context.Context, key, endKey []byte, limit int)
defer span.Finish()
}
start := time.Now()
defer cmdDurationScanRegions.Observe(time.Since(start).Seconds())
defer func() { cmdDurationScanRegions.Observe(time.Since(start).Seconds()) }()

var cancel context.CancelFunc
scanCtx := ctx
Expand Down
66 changes: 50 additions & 16 deletions server/api/region.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,13 @@ func (h *regionHandler) GetRegionByID(w http.ResponseWriter, r *http.Request) {
}

regionInfo := rc.GetRegion(regionID)
h.rd.JSON(w, http.StatusOK, NewAPIRegionInfo(regionInfo))
b, err := marshalRegionInfoJSON(r.Context(), regionInfo)
if err != nil {
h.rd.JSON(w, http.StatusInternalServerError, err.Error())
return
}

h.rd.Data(w, http.StatusOK, b)
}

// @Tags region
Expand All @@ -276,7 +282,13 @@ func (h *regionHandler) GetRegion(w http.ResponseWriter, r *http.Request) {
return
}
regionInfo := rc.GetRegionByKey([]byte(key))
h.rd.JSON(w, http.StatusOK, NewAPIRegionInfo(regionInfo))
b, err := marshalRegionInfoJSON(r.Context(), regionInfo)
if err != nil {
h.rd.JSON(w, http.StatusInternalServerError, err.Error())
return
}

h.rd.Data(w, http.StatusOK, b)
}

// @Tags region
Expand Down Expand Up @@ -336,6 +348,24 @@ func newRegionsHandler(svr *server.Server, rd *render.Render) *regionsHandler {
}
}

// marshalRegionInfoJSON marshals region to bytes in `RegionInfo`'s JSON format.
// It is used to reduce the cost of JSON serialization.
func marshalRegionInfoJSON(ctx context.Context, r *core.RegionInfo) ([]byte, error) {
out := &jwriter.Writer{}

region := &RegionInfo{}
select {
case <-ctx.Done():
// Return early, avoid the unnecessary computation.
// See more details in https://github.com/tikv/pd/issues/6835
return nil, ctx.Err()
default:
}

covertAPIRegionInfo(r, region, out)
return out.Buffer.BuildBytes(), out.Error
}

// marshalRegionsInfoJSON marshals regions to bytes in `RegionsInfo`'s JSON format.
// It is used to reduce the cost of JSON serialization.
func marshalRegionsInfoJSON(ctx context.Context, regions []*core.RegionInfo) ([]byte, error) {
Expand All @@ -359,27 +389,31 @@ func marshalRegionsInfoJSON(ctx context.Context, regions []*core.RegionInfo) ([]
if i > 0 {
out.RawByte(',')
}
InitRegion(r, region)
// EasyJSON will not check anonymous struct pointer field and will panic if the field is nil.
// So we need to set the field to default value explicitly when the anonymous struct pointer is nil.
region.Leader.setDefaultIfNil()
for i := range region.Peers {
region.Peers[i].setDefaultIfNil()
}
for i := range region.PendingPeers {
region.PendingPeers[i].setDefaultIfNil()
}
for i := range region.DownPeers {
region.DownPeers[i].setDefaultIfNil()
}
region.MarshalEasyJSON(out)
covertAPIRegionInfo(r, region, out)
}
out.RawByte(']')

out.RawByte('}')
return out.Buffer.BuildBytes(), out.Error
}

func covertAPIRegionInfo(r *core.RegionInfo, region *RegionInfo, out *jwriter.Writer) {
InitRegion(r, region)
// EasyJSON will not check anonymous struct pointer field and will panic if the field is nil.
// So we need to set the field to default value explicitly when the anonymous struct pointer is nil.
region.Leader.setDefaultIfNil()
for i := range region.Peers {
region.Peers[i].setDefaultIfNil()
}
for i := range region.PendingPeers {
region.PendingPeers[i].setDefaultIfNil()
}
for i := range region.DownPeers {
region.DownPeers[i].setDefaultIfNil()
}
region.MarshalEasyJSON(out)
}

// @Tags region
// @Summary List all regions in the cluster.
// @Produce json
Expand Down
8 changes: 4 additions & 4 deletions server/api/rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (suite *ruleTestSuite) TestSet() {
}

func (suite *ruleTestSuite) TestGet() {
rule := placement.Rule{GroupID: "a", ID: "20", StartKeyHex: "1111", EndKeyHex: "3333", Role: "voter", Count: 1}
rule := &placement.Rule{GroupID: "a", ID: "20", StartKeyHex: "1111", EndKeyHex: "3333", Role: "voter", Count: 1}
data, err := json.Marshal(rule)
suite.NoError(err)
re := suite.Require()
Expand All @@ -194,7 +194,7 @@ func (suite *ruleTestSuite) TestGet() {

testCases := []struct {
name string
rule placement.Rule
rule *placement.Rule
found bool
code int
}{
Expand All @@ -206,7 +206,7 @@ func (suite *ruleTestSuite) TestGet() {
},
{
name: "not found",
rule: placement.Rule{GroupID: "a", ID: "30", StartKeyHex: "1111", EndKeyHex: "3333", Role: "voter", Count: 1},
rule: &placement.Rule{GroupID: "a", ID: "30", StartKeyHex: "1111", EndKeyHex: "3333", Role: "voter", Count: 1},
found: false,
code: 404,
},
Expand All @@ -217,7 +217,7 @@ func (suite *ruleTestSuite) TestGet() {
url := fmt.Sprintf("%s/rule/%s/%s", suite.urlPrefix, testCase.rule.GroupID, testCase.rule.ID)
if testCase.found {
err = tu.ReadGetJSON(re, testDialClient, url, &resp)
suite.compareRule(&resp, &testCase.rule)
suite.compareRule(&resp, testCase.rule)
} else {
err = tu.CheckGetJSON(testDialClient, url, nil, tu.Status(re, testCase.code))
}
Expand Down
53 changes: 53 additions & 0 deletions tests/pdctl/region/region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,56 @@ func TestRegion(t *testing.T) {
{core.HexRegionKeyStr(r5.GetEndKey()), ""},
}, *rangeHoles)
}

func TestRegionNoLeader(t *testing.T) {
re := require.New(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
cluster, err := tests.NewTestCluster(ctx, 1)
re.NoError(err)
err = cluster.RunInitialServers()
re.NoError(err)
cluster.WaitLeader()
url := cluster.GetConfig().GetClientURL()
stores := []*metapb.Store{
{
Id: 1,
State: metapb.StoreState_Up,
LastHeartbeat: time.Now().UnixNano(),
},
{
Id: 2,
State: metapb.StoreState_Up,
LastHeartbeat: time.Now().UnixNano(),
},
{
Id: 3,
State: metapb.StoreState_Up,
LastHeartbeat: time.Now().UnixNano(),
},
}

leaderServer := cluster.GetServer(cluster.GetLeader())
re.NoError(leaderServer.BootstrapCluster())
for i := 0; i < len(stores); i++ {
pdctl.MustPutStore(re, leaderServer.GetServer(), stores[i])
}

metaRegion := &metapb.Region{
Id: 100,
StartKey: []byte(""),
EndKey: []byte(""),
Peers: []*metapb.Peer{
{Id: 1, StoreId: 1},
{Id: 5, StoreId: 2},
{Id: 6, StoreId: 3}},
RegionEpoch: &metapb.RegionEpoch{ConfVer: 1, Version: 1},
}
r := core.NewRegionInfo(metaRegion, nil)

leaderServer.GetRaftCluster().GetBasicCluster().SetRegion(r)

cmd := pdctlCmd.GetRootCmd()
_, err = pdctl.ExecuteCommand(cmd, "-u", url, "region", "100")
re.NoError(err)
}
Loading