-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
server: Add table range api to http status server #20456
Conversation
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
/rebuild |
Please follow PR Title Format:
Or if the count of mainly changed packages are more than 3, use
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
/rebuild |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM.
err = decoder.Decode(&data) | ||
c.Assert(err, IsNil) | ||
c.Assert(data.TableName, Equals, "t") | ||
c.Assert(len(data.Indices), Equals, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the indices name should also be asserted here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert added
@@ -320,6 +337,30 @@ func (ts *HTTPHandlerTestSuite) TestListTableRegions(c *C) { | |||
c.Assert(err, IsNil) | |||
} | |||
|
|||
func (ts *HTTPHandlerTestSuite) TestListTableRanges(c *C) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge this test into TestRangesAPI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's copied from TestListTableRegions which turns out to be a test for not existing table and a partitioned table which indeed is different test.
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
return | ||
} | ||
|
||
writeData(w, createTableRanges(meta.ID, meta.Name.String(), meta.Indices)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same API will return different data structure responses for the partitioned tables. Can we use []*TableRanges{createTableRanges(meta.ID, meta.Name.String(), meta.Indices)}
to make the API users use it friendly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
/merge |
/run-all-tests |
Signed-off-by: Xiaoguang Sun sunxiaoguang@zhihu.com
What problem does this PR solve?
Add table range api to http status server so users can easily query misc ranges information of a particular table.
What is changed and how it works?
Add /tables/{db}/{table}/ranges to http status server and return ranges information row and all indices of the table.
What's Changed:
Add /tables/{db}/{table}/ranges to http status server.
How it Works:
Compute row ranges based on table's id, indices ranges from table's metadata.
Related changes
Add routing information to server/http_status.go
Add handler to server/http_handler.go
Tests
server/http_handler_test.go
TestRangesAPI
TestListTableRanges
Side effects
None
Release note