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.

Sign up for GitHub

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

Jump to bottom

remove legacy tokenSlot of resource group bucket #7346

Open
guo-shaoge opened this issue Nov 10, 2023 · 2 comments · Fixed by #7344
Open

remove legacy tokenSlot of resource group bucket #7346

guo-shaoge opened this issue Nov 10, 2023 · 2 comments · Fixed by #7344
Labels
affects-7.1 affects-7.5 severity/minor type/bug The issue is confirmed as a bug.

Comments

@guo-shaoge
Copy link
Contributor

Enhancement Task

Normally when node step out gracefully(got SIGTERM etc), it will send last acquire grpc, so PD will cleanup its slot token.

But when node crash unexpctedly or got SIGKILL(which cannot be handled), it will have no chance to send last acquire grpc, so its corresponding slot token will leave here forever, which makes fillrate/burstLimit be less.
@guo-shaoge guo-shaoge added the type/enhancement The issue or PR belongs to an enhancement. label Nov 10, 2023
@guo-shaoge guo-shaoge mentioned this issue Nov 10, 2023
Merged
@nolouch nolouch added type/bug The issue is confirmed as a bug. severity/major and removed type/enhancement The issue or PR belongs to an enhancement. labels Nov 10, 2023
@ti-chi-bot ti-chi-bot bot added the needs-type Indicates an issue requires a type label. label Nov 10, 2023
@nolouch nolouch added type/bug The issue is confirmed as a bug. severity/major affects-7.1 affects-7.5 and removed needs-type Indicates an issue requires a type label. labels Nov 10, 2023
@CabinfeverB CabinfeverB added needs-type Indicates an issue requires a type label. type/bug The issue is confirmed as a bug. and removed type/bug The issue is confirmed as a bug. may-affects-5.3 may-affects-5.4 may-affects-6.1 may-affects-6.5 affects-7.1 affects-7.5 labels Nov 10, 2023
@ti-chi-bot ti-chi-bot bot removed the needs-type Indicates an issue requires a type label. label Nov 10, 2023
@CabinfeverB
Copy link
Member

IMO, the weight we use is a quadratic function, if the load gap between clients is too large, it will still give too much to the low-load client. 0 is a limiting case.
We can solve some problems with 7344 first, but there may still be problems with other scenarios.

ti-chi-bot bot pushed a commit that referenced this issue Nov 10, 2023
@guo-shaoge
mcs/resourcemanager: delete expire tokenSlot (#7344)
f1cee6c 
close #7346

Signed-off-by: guo-shaoge <[email protected]>
@ti-chi-bot ti-chi-bot mentioned this issue Nov 10, 2023
Merged
@CabinfeverB CabinfeverB reopened this Nov 10, 2023
@ti-chi-bot ti-chi-bot mentioned this issue Nov 10, 2023
Merged
@CabinfeverB
Copy link
Member

after #7344, change it to minor

ti-chi-bot bot pushed a commit that referenced this issue Nov 10, 2023
@ti-chi-bot @guo-shaoge
mcs/resourcemanager: delete expire tokenSlot (#7344) (#7350)
d09a4f5 
close #7346

Signed-off-by: guo-shaoge <[email protected]>

Co-authored-by: guo-shaoge <[email protected]>
ti-chi-bot bot added a commit that referenced this issue Dec 11, 2023
@ti-chi-bot @guo-shaoge @ti-chi-bot
mcs/resourcemanager: delete expire tokenSlot (#7344) (#7349)
c5d6f3f 
close #7346

Signed-off-by: guo-shaoge <[email protected]>

Co-authored-by: guo-shaoge <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
@HuSharp HuSharp mentioned this issue Mar 4, 2024
Merged
18 tasks
@qiancai qiancai mentioned this issue Apr 23, 2024
Merged
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects-7.1 affects-7.5 severity/minor type/bug The issue is confirmed as a bug.
Projects
Questions and Bug Reports
Status: Need Triage
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mcs/resourcemanager: delete expire tokenSlot guo-shaoge/pd
3 participants
@nolouch @guo-shaoge @CabinfeverB