[cherry-pick 6327] server: add accelerate-schedule/batch api #6348

Lloyd-Pottiger · 2023-04-20T04:04:48Z

Thank you for working on PD! Please read PD's CONTRIBUTING document BEFORE filing this PR.

PR Title Format:

pkg [, pkg2, pkg3]: what's changed
*: what's changed

-->

What problem does this PR solve?

Issue Number: Close #6326

What is changed and how does it work?

add accelerate-schedule/batch api

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Code changes

Has the configuration change
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2023-04-20T04:04:50Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bufferflies
lhy1024

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2023-04-20T04:04:57Z

Hi @Lloyd-Pottiger. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2023-04-20T04:21:11Z

Codecov Report

Patch coverage: 78.22% and project coverage change: +0.02 🎉

Comparison is base (7910e2c) 75.65% compared to head (4ba5c21) 75.67%.

❗ Current head 4ba5c21 differs from pull request most recent head 154d785. Consider uploading reports for the commit 154d785 to get more accurate results

Additional details and impacted files

@@               Coverage Diff               @@
##           release-6.5    #6348      +/-   ##
===============================================
+ Coverage        75.65%   75.67%   +0.02%     
===============================================
  Files              330      330              
  Lines            32975    33009      +34     
===============================================
+ Hits             24946    24980      +34     
+ Misses            5881     5876       -5     
- Partials          2148     2153       +5

Flag	Coverage Δ
unittests	`75.67% <78.22%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/api/region.go	`68.21% <51.51%> (-1.32%)`	⬇️
server/cluster/unsafe_recovery_controller.go	`82.96% <87.77%> (+4.25%)`	⬆️
server/api/router.go	`97.96% <100.00%> (+<0.01%)`	⬆️

... and 23 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

bufferflies · 2023-04-21T11:36:41Z

server/api/region.go

+// @Success  200  {string}  string  "Accelerate regions scheduling in given ranges [startKey1, endKey1), [startKey2, endKey2), ..."
+// @Failure  400  {string}  string  "The input is invalid."
+// @Router   /regions/accelerate-schedule/batch [post]
+func (h *regionsHandler) AccelerateRegionsScheduleInRanges(w http.ResponseWriter, r *http.Request) {


how about exract the same code as function from AccelerateRegionsScheduleInRange?

AccelerateRegionsScheduleInRange is not used anymore, maybe we can remove it later.

I think the old api can't be remove, you don't know the other compoment has depends on it .

bufferflies · 2023-04-21T11:39:44Z

server/api/region.go

+	var msgBuilder strings.Builder
+	msgBuilder.Grow(128)
+	msgBuilder.WriteString("Accelerate regions scheduling in given ranges: ")
+	var regions []*core.RegionInfo


why not use regionsIDList?

bufferflies · 2023-04-21T11:40:32Z

server/api/region.go

+			h.rd.JSON(w, http.StatusBadRequest, err.Error())
+			return
+		}
+		regions = append(regions, rc.ScanRegions(startKey, endKey, limit)...)


what's happened if the regions exist this new region?

not possible, this api is used by partition table, the regions come from different partition tables.

but this api has been expose by all people. unless you can ensure only the partition table use it .

It is only used by partition table now.
To avoid this, change to use set.

It's my mistake, not need to use set, cache TTL is map

pd/pkg/cache/ttl.go

Lines 65 to 68 in 5bdfa71

c.items[key] = ttlCacheItem{

value: value,

expire: time.Now().Add(ttl),

}

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

bufferflies

the reset LGTM

lhy1024 · 2023-04-24T07:09:13Z

/ok-to-test

lhy1024 · 2023-04-24T07:10:01Z

/merge

ti-chi-bot · 2023-04-24T07:10:03Z

@lhy1024: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2023-04-24T07:10:05Z

This pull request has been accepted and is ready to merge.

Commit hash: 154d785

server: add accelerate-schedule/batch api

c724578

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/cherry-pick-not-approved needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Apr 20, 2023

ti-chi-bot requested review from JmPotato and nolouch April 20, 2023 04:04

bufferflies reviewed Apr 21, 2023

View reviewed changes

address comments

154d785

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

Lloyd-Pottiger force-pushed the release-6.5-add-batch-accelerate-schedule-api branch from 4ba5c21 to 154d785 Compare April 23, 2023 03:37

bufferflies reviewed Apr 23, 2023

View reviewed changes

VelocityLight added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Apr 24, 2023

ti-chi-bot bot deleted a comment from ti-chi-bot Apr 24, 2023

bufferflies approved these changes Apr 24, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 24, 2023

ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Apr 24, 2023

lhy1024 approved these changes Apr 24, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 24, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 24, 2023

ti-chi-bot bot merged commit 10b691a into tikv:release-6.5 Apr 24, 2023

ti-chi-bot removed the cherry-pick-approved Cherry pick PR approved by release team. label Jun 14, 2023

ti-chi-bot bot added the do-not-merge/cherry-pick-not-approved label Jun 14, 2023

ti-chi-bot added the cherry-pick-approved Cherry pick PR approved by release team. label Jun 14, 2023

ti-chi-bot bot removed the do-not-merge/cherry-pick-not-approved label Jun 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cherry-pick 6327] server: add accelerate-schedule/batch api #6348

[cherry-pick 6327] server: add accelerate-schedule/batch api #6348

Lloyd-Pottiger commented Apr 20, 2023

ti-chi-bot commented Apr 20, 2023 •

edited by ti-chi-bot bot

Loading

ti-chi-bot commented Apr 20, 2023

codecov bot commented Apr 20, 2023 •

edited

Loading

bufferflies Apr 21, 2023

Lloyd-Pottiger Apr 23, 2023

bufferflies Apr 23, 2023

bufferflies Apr 21, 2023

Lloyd-Pottiger Apr 23, 2023

bufferflies Apr 21, 2023

Lloyd-Pottiger Apr 23, 2023

bufferflies Apr 23, 2023

Lloyd-Pottiger Apr 23, 2023

bufferflies Apr 23, 2023 •

edited

Loading

Lloyd-Pottiger Apr 23, 2023

bufferflies left a comment

lhy1024 commented Apr 24, 2023

lhy1024 commented Apr 24, 2023

ti-chi-bot bot commented Apr 24, 2023

ti-chi-bot bot commented Apr 24, 2023

	c.items[key] = ttlCacheItem{
	value: value,
	expire: time.Now().Add(ttl),
	}

[cherry-pick 6327] server: add accelerate-schedule/batch api #6348

[cherry-pick 6327] server: add accelerate-schedule/batch api #6348

Conversation

Lloyd-Pottiger commented Apr 20, 2023

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Apr 20, 2023 • edited by ti-chi-bot bot Loading

ti-chi-bot commented Apr 20, 2023

codecov bot commented Apr 20, 2023 • edited Loading

Codecov Report

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

bufferflies Apr 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bufferflies left a comment

Choose a reason for hiding this comment

lhy1024 commented Apr 24, 2023

lhy1024 commented Apr 24, 2023

ti-chi-bot bot commented Apr 24, 2023

ti-chi-bot bot commented Apr 24, 2023

ti-chi-bot commented Apr 20, 2023 •

edited by ti-chi-bot bot

Loading

codecov bot commented Apr 20, 2023 •

edited

Loading

bufferflies Apr 23, 2023 •

edited

Loading