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

Fix bugs of throttling control behavior in flow control module #332

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Nov 25, 2020

Describe what this PR does / why we need it

Fix bugs of throttling control behavior in flow control module, including overflow bug and time unit bug of wait duration.

Does this pull request fix one issue?

Fixes #331

Describe how you did it

  • Change semantic of waitMs to nanosToWait in TokenResult and polish related stat slots. Here we SHOULD use nanos instead of millis, or the queueing interval may not be accurate.
  • Fix the bug that unsigned estimatedQueueingDuration in throttling checker may overflow. Use int64 instead.

Describe how to verify it

Run the test cases and demo.

Special notes for reviews

NONE.

@sczyh30 sczyh30 added kind/bug Something isn't working kind/enhancement Category issues or PRs related to enhancement area/flow-control Issues or PRs related to flow control labels Nov 25, 2020
@sczyh30 sczyh30 requested a review from louyuting November 25, 2020 15:37
@sczyh30
Copy link
Member Author

sczyh30 commented Nov 25, 2020

Also cc @NineSunRD @luckyxiaoqiang @sanxun0325

core/base/result.go Outdated Show resolved Hide resolved
…cker may overflow

* Use int64 instead of uint64 here
* Update wait duration to nanos

Signed-off-by: Eric Zhao <[email protected]>
@sczyh30 sczyh30 force-pushed the fix-throttling-overflow branch from 6a0224b to d443d80 Compare November 26, 2020 02:28
Copy link
Collaborator

@louyuting louyuting left a comment

Choose a reason for hiding this comment

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

Nice work~

@sczyh30 sczyh30 merged commit 963192c into alibaba:master Nov 26, 2020
@sczyh30 sczyh30 deleted the fix-throttling-overflow branch November 26, 2020 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flow-control Issues or PRs related to flow control kind/bug Something isn't working kind/enhancement Category issues or PRs related to enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] estimatedQueueingDuration in ThrottlingChecker may overflow, which may cause infinite waiting
2 participants