diff --git a/client/client.go b/client/client.go index daf3eac7968..f0d51ef2596 100644 --- a/client/client.go +++ b/client/client.go @@ -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 diff --git a/server/api/region.go b/server/api/region.go index 4e6679a376d..e81d4f0b1db 100644 --- a/server/api/region.go +++ b/server/api/region.go @@ -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 @@ -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 @@ -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) { @@ -359,20 +389,7 @@ 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(']') @@ -380,6 +397,23 @@ func marshalRegionsInfoJSON(ctx context.Context, regions []*core.RegionInfo) ([] 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 diff --git a/server/api/rule_test.go b/server/api/rule_test.go index d2000eb9562..8233e96f463 100644 --- a/server/api/rule_test.go +++ b/server/api/rule_test.go @@ -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() @@ -194,7 +194,7 @@ func (suite *ruleTestSuite) TestGet() { testCases := []struct { name string - rule placement.Rule + rule *placement.Rule found bool code int }{ @@ -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, }, @@ -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)) } diff --git a/tests/pdctl/region/region_test.go b/tests/pdctl/region/region_test.go index d56463d728d..5e381158672 100644 --- a/tests/pdctl/region/region_test.go +++ b/tests/pdctl/region/region_test.go @@ -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) +}