http: get region's info #2774

AndreMouche · 2017-03-03T09:45:26Z

Hi,all,
This PR is used to list regions for table && get region by ID through http request.

Details:

List table's Regions

Requests

Syntax

GET /tables/${db}/${table}/regions HTTP/1.1
Host: status.tidb.domain
Date: date

Request Parameters

Name	Description	Required
db	database's name	YES
table	table's name	YES

Response

Response Elements

Parent	Name	Type	Description
－	name	string	table's name
－	id	int	table's ID
－	record_regions	[]object	regions which table's record key convered
－	indices	[]object	list of table's indices
indices[]	name	string	index's name
indices[]	id	int	index's ID
indices[]	regions	[]object	regions which current index covered

Response Sample

HTTP/1.1 200 OK
Content-Length:244
Content-Type:application/json
Date:Tue, 07 Mar 2017 07:50:31 GMT
{
    "name": "mcc_access",
    "id": 19,
    "record_regions": [
        {
            "region_id": 7,
            "leader": {
                "id": 10,
                "store_id": 1
            },
            "peers": [
                {
                    "id": 8,
                    "store_id": 2
                },
                {
                    "id": 10,
                    "store_id": 1
                }
            ],
            "region_epoch": {
                "conf_ver": 2,
                "version": 2
            }
        }
    ],
    "indices": [
        {
            "name": "access_key",
            "id": 1,
            "regions": [
                {
                    "region_id": 7,
                    "leader": {
                        "id": 10,
                        "store_id": 1
                    },
                    "peers": [
                        {
                            "id": 8,
                            "store_id": 2
                        },
                        {
                            "id": 10,
                            "store_id": 1
                        }
                    ],
                    "region_epoch": {
                        "conf_ver": 2,
                        "version": 2
                    }
                }
            ]
        }
    ]
}

GET Region By ID

Requests

Syntax

GET /regions/${regionID} HTTP/1.1
Host: status.tidb.domain
Date: date

Request Parameters

Name	Description	Required
regionID	ID of region to get	YES

Response

Response Elements

Parent	Name	Type	Description
－	region_id	int	region's id
－	start_key	string	region's start_key
--	end_key	string	region's end_key
－	frames	[]object	list of frames current region covered
frames[]	table_name	string	table's name
frames[]	table_id	int	table's ID
frames[]	is_record	bool	whether current frame is a record
frames[]	index_name	string	index's name
frames[]	index_id	int	index's ID

Response Sample

HTTP/1.1 200 OK
Content-Length:244
Content-Type:application/json
Date:Tue, 07 Mar 2017 07:50:31 GMT
{
  region_id: 7,
  start_key: "bURETEpvYkj/af9zdG9yeQD/AAD8AAAAAAD/AABoAAAAAAD/AAAG/wAAAAD/AAAAAPcAAAD8",
  end_key: "",
  frames: [
      {
      db_name: "INFORMATION_SCHEMA",
      table_name: "STATISTICS",
      table_id: 9223372036854776000,
      is_record: true
      },
      {
        db_name: "test",
        table_name: "mcc_access",
        table_id: 19,
        is_record: false,
        index_name: "access_key",
        index_id: 1
      }
  ]
}

siddontang · 2017-03-04T05:41:21Z

server/server.go

@@ -250,6 +251,18 @@ func (s *Server) startStatusHTTP() {
 			})
 			// HTTP path for prometheus.
 			http.Handle("/metrics", prometheus.Handler())
+			// default path for regions status info


use /regions

shenli · 2017-03-05T02:18:16Z

@AndreMouche CI failed.

coocood

Need test for region handler.

coocood · 2017-03-05T01:20:07Z

server/region_handler.go

+	"github.com/pingcap/tidb/store/tikv"
+	"github.com/pingcap/tidb/tablecodec"
+	"github.com/pingcap/tidb/util/codec"
+	gContext "golang.org/x/net/context"


goctx "context"

coocood · 2017-03-06T03:08:28Z

server/region_handler.go

+	gContext "golang.org/x/net/context"
+	"math"
+	"net/http"
+	"time"


Take a look at how other files in TiDB organize import paths.

coocood · 2017-03-06T03:22:19Z

server/server.go

@@ -39,6 +39,7 @@ import (
 	// For pprof
 	_ "net/http/pprof"

+	"github.com/gorilla/mux"


This adds a dependency, we should add this repo in vendor.

coocood · 2017-03-06T03:30:09Z

server/region_handler.go

+	}
+}
+
+func (req *RegionsHTTPRequest) showResponse(data interface{}, err error) {


Why not name it writeResponse?

coocood · 2017-03-06T03:37:37Z

store/tikv/region_cache.go

@@ -163,6 +182,23 @@ func (c *RegionCache) GroupKeysByRegion(bo *Backoffer, keys [][]byte) (map[Regio
 	return groups, first, nil
 }

+// ListRegionIDsInKeyRange lists ids of regions in [start_key,end_key].
+func (c *RegionCache) ListRegionIDsInKeyRange(bo *Backoffer, startKey, endKey []byte) (regions []uint64, err error) {
+	regions = make([]uint64, 0, 0)


Why make a slice with zero capacity?

The variable name should be regionIDs.

coocood · 2017-03-06T03:46:04Z

store/tikv/region_cache.go

+
+	pdCli, err := pd.NewClient(etcdAddrs)
+	if err != nil {
+		if strings.Contains(err.Error(), "i/o timeout") {


This can be removed, it is useful only for server initialization.

coocood · 2017-03-06T03:48:24Z

store/tikv/region_cache.go

@@ -24,6 +24,7 @@ import (
 	"github.com/pingcap/kvproto/pkg/kvrpcpb"
 	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pingcap/pd/pd-client"
+	"strings"


import path ordering.

coocood · 2017-03-06T03:52:51Z

server/region_handler.go

+		TableName:  tableName,
+		TableID:    tableID,
+		RowRegions: nil,
+		Indices:    make([]IndexRegions, len(table.Indices()), len(table.Indices())),


The third parameter in make is not needed.

siddontang · 2017-03-06T09:53:29Z

server/server.go

-				request.Handle()
+			router.Handle("/metrics", prometheus.Handler())
+
+			router.HandleFunc("/tables/{db}/{table}/regions", func(w http.ResponseWriter, req *http.Request) {


I prefer /regions/db/table

… or end_key is not index key

…s too big

AndreMouche · 2017-03-14T09:23:14Z

PTAL

siddontang · 2017-03-14T12:11:12Z

server/region_handler.go

+}
+
+// HTTPListTableRegionsHandler is the handler for list table's regions.
+type HTTPListTableRegionsHandler struct {


TableRegionsHandler

siddontang · 2017-03-14T12:26:55Z

server/region_handler.go

+// HTTPRegionHandler is the common field for http region handler. It contains
+// some common functions which would be used in HTTPListTableRegionsHandler and
+// HTTPGetRegionByIDHandler.
+type HTTPRegionHandler struct {


You can use RegionHandler to handle /regions/{regionID} and TableRegionsHandler to handle /tables/db/table

…ot in range

AndreMouche · 2017-03-15T13:43:15Z

@coocood PTAL

siddontang · 2017-03-16T07:21:38Z

server/region_handler.go

+	// get from table's ID directly. Above all, here do dot process like
+	// 		`for id in [frameRange.firstTableID,frameRange.endTableID]`
+	// on [frameRange.firstTableID,frameRange.endTableID] is small enough.
+	for _, db := range tool.infoSchema.AllSchemas() {


here we will add all databases and tables? this may cost lots of memory.

@siddontang Since we need a database's name for each frame, and a table's database name can not get from table's ID directly. So here do dot process like

for id in [frameRange.firstTableID,frameRange.endTableID] ...

on [frameRange.firstTableID,frameRange.endTableID] is small enough.

Above all, it seems iterate all databases and tables cannot be avoided. While with infoSchema already have all databases with tables in memory, there is no additional memory cost.

can we add a function like GetTableByID in infoshema? @coocood

@siddontang We have GetTableByID, but the table doesn't know about the DB name.

coocood · 2017-03-16T08:53:55Z

LGTM

siddontang · 2017-03-16T09:00:58Z

PTAL @disksing

disksing · 2017-03-15T06:33:31Z

store/tikv/region_cache.go

@@ -163,6 +192,22 @@ func (c *RegionCache) GroupKeysByRegion(bo *Backoffer, keys [][]byte) (map[Regio
 	return groups, first, nil
 }

+// ListRegionIDsInKeyRange lists ids of regions in [start_key,end_key].
+func (c *RegionCache) ListRegionIDsInKeyRange(bo *Backoffer, startKey, endKey []byte) (regionIDs []uint64, err error) {
+	for err == nil {


err will never be nil.

disksing · 2017-03-16T10:00:16Z

server/region_handler_test.go

+// distributed under the License is distributed on an "AS IS" BASIS,
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// +build !race


Why disable race detector?

disksing · 2017-03-16T10:08:45Z

server/region_handler.go

+}
+
+// addIndex insert a index into RegionDetail.
+func (rt *RegionDetail) addIndex(dbName, tName string, tID int64, indexName string, indexID int64) {


Please move the methods to the place next to the structure.

disksing · 2017-03-16T10:13:41Z

server/region_handler.go

+// TableRegionsHandler is the handler for list table's regions.
+type TableRegionsHandler struct {
+	RegionHandler
+}


I think we don't need TableRegionsHandler. How about define 2 methods on RegionHandler and use HandleFunc to register them to router?

disksing · 2017-03-16T10:15:41Z

server/region_handler.go

+	}
+}
+
+func (rh TableRegionsHandler) getRegionsMetaWithRegionIds(rIDs []uint64) ([]RegionMeta, error) {


Id should be ID. Also I think just getRegionMetas(regionIDs []uint64) is ok.

disksing · 2017-03-16T10:19:30Z

server/region_handler.go

+// prepare checks and prepares for region request. It returns
+// regionHandlerTool on success while return an err on any
+// error happens.
+func (rh *RegionHandler) prepare() (tool *regionHandlerTool, err error) {


Seems regionHandlerTool is redundant. I think it would be more clear to return infoSchema, regionCache, error.

It may cause too many params for the func? And make regionHandlerTool as they are common tools used in region handler. @disksing

disksing · 2017-03-16T10:21:32Z

server/region_handler.go

+	if rh.pdClient == nil {
+		err = fmt.Errorf("Invalid PdClient: nil")
+		return
+	}


I suppose it won't be nil.

disksing · 2017-03-16T12:12:52Z

store/tikv/kv.go

+func ParsePath(path string) (etcdAddrs []string, disableGC bool, err error) {
+	return parsePath(path)
+}
+


Why not just rename 'parsePath' to 'ParsePath'?

Because too many function are using parsePath. I also think ParsePath is not a good name used as public function and would like to change someday. @disksing

disksing · 2017-03-16T12:15:14Z

tablecodec/tablecodec_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pingcap/tidb/util/codec"
 	"github.com/pingcap/tidb/util/testleak"
 	"github.com/pingcap/tidb/util/types"
+	"math"


The imports are not in good order. We should separate std packages from 3rd-party packages.

disksing · 2017-03-16T12:22:08Z

store/tikv/region_cache_test.go

+
+	regionIDs, err := s.cache.ListRegionIDsInKeyRange(s.bo, []byte("a"), []byte("z"))
+	c.Assert(err, IsNil)
+	c.Assert(len(regionIDs), Equals, 2)


Why only check length? I suppose you can check the IDs are correct.
c.Assert(regionIDs, DeeqEquals, []uint64{s.region1, region2})

disksing · 2017-03-16T12:37:46Z

server/region_handler_test.go

+	resp, err := http.Get("http://127.0.0.1:10090/tables/information_schema/SCHEMATA/regions")
+	c.Assert(err, IsNil)
+
+	//c.Assert(resp.StatusCode,Equals,http.StatusOK)


Why comment it?

disksing · 2017-03-16T12:43:37Z

server/region_handler_test.go

+	err = decoder.Decode(&data)
+	c.Assert(err, IsNil)
+
+	c.Assert(len(data.RecordRegions) > 0, IsTrue)


c.Assert(len(data.RecordRegions), Greater, 0)

AndreMouche · 2017-03-16T12:44:17Z

@BusyJay PTAL

disksing · 2017-03-17T02:26:19Z

LGTM, PTAL @siddontang

AndreMouche added 11 commits March 1, 2017 15:01

regions:init func ListRegionIDsInKeyRange

3990208

regions:init restful api

8dab9b3

regions:adjust code

f236f6a

regions:fix bug && add debug log

2c66f68

regions:fix bug

2a72068

regions:init copyright

95ab9f8

regions:refact

dbea7ca

regions:fix bug

df55c15

regions:reformat code

a8a86e5

regions:reformat code

af33440

Merge branch 'master' into shirly/regions

4910df6

AndreMouche changed the title ~~Shirly/regions~~ Http: list regions for table Mar 4, 2017

siddontang reviewed Mar 4, 2017

View reviewed changes

AndreMouche changed the title ~~Http: list regions for table~~ http: list regions for table Mar 4, 2017

AndreMouche added 2 commits March 4, 2017 15:04

regions:use gorilla/mux for router

198f33f

regions:merge master

43a6e40

Merge branch 'master' into shirly/regions

a3e3102

coocood reviewed Mar 6, 2017

View reviewed changes

AndreMouche added 3 commits March 6, 2017 13:49

regions:init gorilla/mux in glide

14ae269

Merge branch 'master' into shirly/regions

76912d6

regions:adjust code

88fdf4e

AndreMouche changed the title ~~http: list regions for table~~ [WIP]http: list regions for table Mar 6, 2017

siddontang reviewed Mar 6, 2017

View reviewed changes

AndreMouche added 5 commits March 6, 2017 19:31

regions:int get region's info by id through http request

9991169

Merge branch 'master' into shirly/regions

bddaa76

regions:use binary search to find index range when region's start_key…

c13b662

… or end_key is not index key

regions:iterate table id from infoschema when index range of region i…

9077c54

…s too big

regions:remove log for debug and update copyright

6b5bec2

AndreMouche changed the title ~~[WIP]http: list regions for table~~ http: regions Mar 7, 2017

regions:refact

4a7e94f

siddontang reviewed Mar 14, 2017

View reviewed changes

AndreMouche added 4 commits March 14, 2017 21:45

regions:refact for comments

f9952c7

Merge branch 'master' into shirly/regions

56a8c4d

regions:recover pprof and make backoffer maxsleep smaller

b6db841

regions:get region by id--fix bug caused by illegal check for table n…

cc7e27a

…ot in range

AndreMouche added 2 commits March 15, 2017 21:43

Merge branch 'master' into shirly/regions

5ca4ad2

Merge branch 'master' into shirly/regions

7b0dcac

siddontang reviewed Mar 16, 2017

View reviewed changes

disksing reviewed Mar 16, 2017

View reviewed changes

regions:update according to annotation

d002a02

disksing reviewed Mar 16, 2017

View reviewed changes

AndreMouche added 2 commits March 16, 2017 20:40

regions:update according annotations

7a8d3ad

regions:update according annotations

fa638f4

disksing reviewed Mar 16, 2017

View reviewed changes

Merge branch 'master' into shirly/regions

e308528

Merge branch 'master' into shirly/regions

a356c44

disksing approved these changes Mar 17, 2017

View reviewed changes

disksing merged commit e791b9a into master Mar 17, 2017

disksing deleted the shirly/regions branch March 17, 2017 04:09

http: get region's info #2774

http: get region's info #2774

Conversation

AndreMouche commented Mar 3, 2017 • edited Loading

List table's Regions

Requests

Syntax

Request Parameters

Response

Response Elements

Response Sample

GET Region By ID

Requests

Syntax

Request Parameters

Response

Response Elements

Response Sample

Choose a reason for hiding this comment

shenli commented Mar 5, 2017

coocood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMouche commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMouche commented Mar 15, 2017

Choose a reason for hiding this comment

AndreMouche Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coocood commented Mar 16, 2017

siddontang commented Mar 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMouche Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMouche commented Mar 16, 2017

disksing commented Mar 17, 2017

AndreMouche commented Mar 3, 2017 •

edited

Loading

AndreMouche Mar 16, 2017 •

edited

Loading

AndreMouche Mar 16, 2017 •

edited

Loading