Skip to content
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

api: support query regions by read/write flow. #897

Merged
merged 9 commits into from
Jan 10, 2018

Conversation

disksing
Copy link
Contributor

@disksing disksing commented Dec 29, 2017

Query for the top N regions by readFlow/writeFlow.
Usage:
HTTP:

curl http://127.0.0.1:2379/pd/api/v1/regions/{writeflow, readflow}[?limit=:count]

pd-ctl:

> region {topread, topwrite} [:count]

@siddontang
Copy link
Contributor

Please add a PR description.

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

// Pop removes the minimum element (according to Less) from the heap and returns
// it.
func (h *RegionHeap) Pop() interface{} {
pos := len(h.regions) - 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better check whether Len() is zero

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the caller's responsibility to avoid poping an empty heap. You can refer the heap example: https://golang.org/src/container/heap/example_intheap_test.go

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it


const defaultRegionLimit = 16

func (h *regionsHandler) GetTopWriteFlow(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we combine TopRead/WriteFlow? They use nearly same codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. PTAL @siddontang

}

// RegionHeap implements heap.Interface, used for selecting top n regions.
type RegionHeap struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any independent test for RegionHeap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess not necessary. It is only a straightforward adapter to wrap a slice into heap.Interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add some tests for it, we are actually testing the algorithms in golang library.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but here it not only for golang lib, we have min and TopN too.

Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

return h.regions[0]
}

func (h *regionsHandler) TopNRegions(regions []*core.RegionInfo, less func(a, b *core.RegionInfo) bool, n int) []*core.RegionInfo {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use TopN directly here?

return nil
}

hp := &RegionHeap{less: less}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need preallocation here?

@siddontang
Copy link
Contributor

Rest LGTM

@disksing disksing merged commit 2439612 into tikv:master Jan 10, 2018
@disksing disksing deleted the region branch January 10, 2018 02:28
ti-chi-bot pushed a commit that referenced this pull request Feb 15, 2023
…1.11.1 in /tools/pd-tso-bench (#5990)

ref #897, ref #962, ref #969, ref #974, ref #975, ref #976, ref #986, ref prometheus/client_golang#987, ref #987, ref #989, ref #998, ref #1013, ref #1014, ref #1025, ref #1028, ref #1031, ref #1043, ref #1055, ref #1075, ref #1091, ref #1094, ref #1102, ref #1103, ref #1118, ref #1146, ref #1148, ref #1150

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ti-chi-bot pushed a commit that referenced this pull request Feb 15, 2023
…1.11.1 in /tests/client (#5992)

ref #897, ref #962, ref #969, ref #974, ref #975, ref #976, ref #986, ref #987, ref prometheus/client_golang#987, ref #989, ref #998, ref #1013, ref #1014, ref #1025, ref #1028, ref #1031, ref #1043, ref #1055, ref #1075, ref #1091, ref #1094, ref #1102, ref #1103, ref #1118, ref #1146, ref #1148, ref #1150, ref #4399

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ti-chi-bot added a commit that referenced this pull request Feb 15, 2023
…1.11.1 in /tests/mcs (#5993)

ref #897, ref #962, ref #969, ref #974, ref #975, ref #976, ref #986, ref #987, ref prometheus/client_golang#987, ref #989, ref #998, ref #1013, ref #1014, ref #1025, ref #1028, ref #1031, ref #1043, ref #1055, ref #1075, ref #1091, ref #1094, ref #1102, ref #1103, ref #1118, ref #1146, ref #1148, ref #1150, ref #4399

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ti Chi Robot <[email protected]>
ti-chi-bot added a commit that referenced this pull request Feb 15, 2023
…1.11.1 in /client (#5991)

ref #897, ref #962, ref #969, ref #974, ref #975, ref #976, ref #986, ref #987, ref prometheus/client_golang#987, ref #989, ref #998, ref #1013, ref #1014, ref #1025, ref #1028, ref #1031, ref #1043, ref #1055, ref #1075, ref #1091, ref #1094, ref #1102, ref #1103, ref #1118, ref #1146, ref #1148, ref #1150, ref #4399

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ti Chi Robot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants