Skip to content

Commit

Permalink
api: fix panic when region doesn't have a leader (tikv#7629)
Browse files Browse the repository at this point in the history
close tikv#7630

Signed-off-by: Ryan Leung <[email protected]>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
  • Loading branch information
rleungx and ti-chi-bot[bot] authored Jan 2, 2024
1 parent 1df4aa6 commit 5bd84c2
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 16 deletions.
66 changes: 50 additions & 16 deletions server/api/region.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,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 Down Expand Up @@ -286,7 +292,13 @@ func (h *regionHandler) GetRegion(w http.ResponseWriter, r *http.Request) {
}

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 @@ -323,6 +335,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 @@ -346,27 +376,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
53 changes: 53 additions & 0 deletions tools/pd-ctl/tests/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 := pdTests.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.GetLeaderServer()
re.NoError(leaderServer.BootstrapCluster())
for i := 0; i < len(stores); i++ {
pdTests.MustPutStore(re, cluster, 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)

cluster.GetLeaderServer().GetRaftCluster().GetBasicCluster().SetRegion(r)

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

0 comments on commit 5bd84c2

Please sign in to comment.