From e5afc40373eddc20e7a422bc4add054c949f3826 Mon Sep 17 00:00:00 2001 From: nolouch Date: Tue, 6 Nov 2018 19:47:48 +0800 Subject: [PATCH 1/2] server/api: fix region check API interface --- server/api/region.go | 73 ++++++++++++++++----------------------- server/api/region_test.go | 27 +++++++++++++++ 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/server/api/region.go b/server/api/region.go index cfe7d57a2bf..63401c61aac 100644 --- a/server/api/region.go +++ b/server/api/region.go @@ -124,21 +124,25 @@ func newRegionsHandler(svr *server.Server, rd *render.Render) *regionsHandler { } } -func (h *regionsHandler) GetAll(w http.ResponseWriter, r *http.Request) { - cluster := h.svr.GetRaftCluster() - if cluster == nil { - h.rd.JSON(w, http.StatusInternalServerError, server.ErrNotBootstrapped.Error()) - return - } - regions := cluster.GetRegions() +func convertToAPIRegions(regions []*core.RegionInfo) *regionsInfo { regionInfos := make([]*regionInfo, len(regions)) for i, r := range regions { regionInfos[i] = newRegionInfo(r) } - regionsInfo := ®ionsInfo{ + return ®ionsInfo{ Count: len(regions), Regions: regionInfos, } +} + +func (h *regionsHandler) GetAll(w http.ResponseWriter, r *http.Request) { + cluster := h.svr.GetRaftCluster() + if cluster == nil { + h.rd.JSON(w, http.StatusInternalServerError, server.ErrNotBootstrapped.Error()) + return + } + regions := cluster.GetRegions() + regionsInfo := convertToAPIRegions(regions) h.rd.JSON(w, http.StatusOK, regionsInfo) } @@ -164,15 +168,7 @@ func (h *regionsHandler) ScanRegionsByKey(w http.ResponseWriter, r *http.Request limit = maxRegionLimit } regions := cluster.ScanRegionsByKey([]byte(startKey), limit) - - regionInfos := make([]*regionInfo, 0, len(regions)) - for _, region := range regions { - regionInfos = append(regionInfos, newRegionInfo(region)) - } - regionsInfo := ®ionsInfo{ - Count: len(regionInfos), - Regions: regionInfos, - } + regionsInfo := convertToAPIRegions(regions) h.rd.JSON(w, http.StatusOK, regionsInfo) } @@ -190,65 +186,63 @@ func (h *regionsHandler) GetStoreRegions(w http.ResponseWriter, r *http.Request) return } regions := cluster.GetStoreRegions(uint64(id)) - regionInfos := make([]*regionInfo, len(regions)) - for i, r := range regions { - regionInfos[i] = newRegionInfo(r) - } - regionsInfo := ®ionsInfo{ - Count: len(regions), - Regions: regionInfos, - } + regionsInfo := convertToAPIRegions(regions) h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetMissPeerRegions(w http.ResponseWriter, r *http.Request) { handler := h.svr.GetHandler() - res, err := handler.GetMissPeerRegions() + regions, err := handler.GetMissPeerRegions() if err != nil { h.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetExtraPeerRegions(w http.ResponseWriter, r *http.Request) { handler := h.svr.GetHandler() - res, err := handler.GetExtraPeerRegions() + regions, err := handler.GetExtraPeerRegions() if err != nil { h.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetPendingPeerRegions(w http.ResponseWriter, r *http.Request) { handler := h.svr.GetHandler() - res, err := handler.GetPendingPeerRegions() + regions, err := handler.GetPendingPeerRegions() if err != nil { h.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetDownPeerRegions(w http.ResponseWriter, r *http.Request) { handler := h.svr.GetHandler() - res, err := handler.GetDownPeerRegions() + regions, err := handler.GetDownPeerRegions() if err != nil { h.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetIncorrectNamespaceRegions(w http.ResponseWriter, r *http.Request) { handler := h.svr.GetHandler() - res, err := handler.GetIncorrectNamespaceRegions() + regions, err := handler.GetIncorrectNamespaceRegions() if err != nil { h.rd.JSON(w, http.StatusInternalServerError, err.Error()) return } - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } func (h *regionsHandler) GetRegionSiblings(w http.ResponseWriter, r *http.Request) { @@ -325,14 +319,7 @@ func (h *regionsHandler) GetTopNRegions(w http.ResponseWriter, r *http.Request, limit = maxRegionLimit } regions := topNRegions(cluster.GetRegions(), less, limit) - regionInfos := make([]*regionInfo, len(regions)) - for i, r := range regions { - regionInfos[i] = newRegionInfo(r) - } - res := ®ionsInfo{ - Count: len(regions), - Regions: regionInfos, - } + res := convertToAPIRegions(regions) h.rd.JSON(w, http.StatusOK, res) } diff --git a/server/api/region_test.go b/server/api/region_test.go index 40a25a4e8b2..8994c8a8a00 100644 --- a/server/api/region_test.go +++ b/server/api/region_test.go @@ -19,6 +19,8 @@ import ( "net/url" "sort" + "github.com/pingcap/kvproto/pkg/pdpb" + . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/pd/server" @@ -84,6 +86,31 @@ func (s *testRegionSuite) TestRegion(c *C) { c.Assert(r2, DeepEquals, newRegionInfo(r)) } +func (s *testRegionSuite) TestRegionCheck(c *C) { + r := newTestRegionInfo(2, 1, []byte("a"), []byte("b")) + downPeer := &metapb.Peer{Id: 13, StoreId: 2} + r = r.Clone(core.WithAddPeer(downPeer), core.WithDownPeers([]*pdpb.PeerStats{{Peer: downPeer, DownSeconds: 3600}}), core.WithPendingPeers([]*metapb.Peer{downPeer})) + mustRegionHeartbeat(c, s.svr, r) + url := fmt.Sprintf("%s/region/id/%d", s.urlPrefix, r.GetID()) + r1 := ®ionInfo{} + err := readJSONWithURL(url, r1) + c.Assert(err, IsNil) + c.Assert(r1, DeepEquals, newRegionInfo(r)) + + url = fmt.Sprintf("%s/regions/check/%s", s.urlPrefix, "down-peer") + r2 := ®ionsInfo{} + err = readJSONWithURL(url, r2) + c.Assert(err, IsNil) + c.Assert(r2, DeepEquals, ®ionsInfo{Count: 1, Regions: []*regionInfo{newRegionInfo(r)}}) + + url = fmt.Sprintf("%s/regions/check/%s", s.urlPrefix, "pending-peer") + r3 := ®ionsInfo{} + err = readJSONWithURL(url, r3) + c.Assert(err, IsNil) + c.Assert(r3, DeepEquals, ®ionsInfo{Count: 1, Regions: []*regionInfo{newRegionInfo(r)}}) + +} + func (s *testRegionSuite) TestRegions(c *C) { rs := []*core.RegionInfo{ newTestRegionInfo(2, 1, []byte("a"), []byte("b")), From ad6f16f37881a9437d04449e35b694e79a54d99d Mon Sep 17 00:00:00 2001 From: nolouch Date: Wed, 7 Nov 2018 11:22:19 +0800 Subject: [PATCH 2/2] address comments --- server/api/region.go | 4 ++-- server/api/region_test.go | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/server/api/region.go b/server/api/region.go index cf7a11ea355..cb65b509788 100644 --- a/server/api/region.go +++ b/server/api/region.go @@ -317,8 +317,8 @@ func (h *regionsHandler) GetTopNRegions(w http.ResponseWriter, r *http.Request, limit = maxRegionLimit } regions := topNRegions(cluster.GetRegions(), less, limit) - res := convertToAPIRegions(regions) - h.rd.JSON(w, http.StatusOK, res) + regionsInfo := convertToAPIRegions(regions) + h.rd.JSON(w, http.StatusOK, regionsInfo) } // RegionHeap implements heap.Interface, used for selecting top n regions. diff --git a/server/api/region_test.go b/server/api/region_test.go index 8994c8a8a00..53ff9b07610 100644 --- a/server/api/region_test.go +++ b/server/api/region_test.go @@ -19,10 +19,9 @@ import ( "net/url" "sort" - "github.com/pingcap/kvproto/pkg/pdpb" - . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/kvproto/pkg/pdpb" "github.com/pingcap/pd/server" "github.com/pingcap/pd/server/core" ) @@ -108,7 +107,6 @@ func (s *testRegionSuite) TestRegionCheck(c *C) { err = readJSONWithURL(url, r3) c.Assert(err, IsNil) c.Assert(r3, DeepEquals, ®ionsInfo{Count: 1, Regions: []*regionInfo{newRegionInfo(r)}}) - } func (s *testRegionSuite) TestRegions(c *C) {