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

Refine GetResRule API and GetRules API in flow/circuitbreaker/hotspot/system module #239

Merged
merged 4 commits into from
Sep 14, 2020

Conversation

louyuting
Copy link
Collaborator

Describe what this PR does / why we need it

Refine GetResRule func and GetRules func in flow/circuitbreaker/hotspot/system module

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

@louyuting louyuting requested a review from sczyh30 September 12, 2020 19:05
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2020

Codecov Report

Merging #239 into master will decrease coverage by 0.28%.
The diff coverage is 4.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #239      +/-   ##
==========================================
- Coverage   42.93%   42.64%   -0.29%     
==========================================
  Files          74       74              
  Lines        4316     4345      +29     
==========================================
  Hits         1853     1853              
- Misses       2230     2259      +29     
  Partials      233      233              
Impacted Files Coverage Δ
core/circuitbreaker/rule_manager.go 46.30% <0.00%> (-2.66%) ⬇️
core/circuitbreaker/slot.go 0.00% <0.00%> (ø)
core/circuitbreaker/stat_slot.go 0.00% <0.00%> (ø)
core/flow/rule_manager.go 60.58% <0.00%> (ø)
core/hotspot/rule_manager.go 56.45% <0.00%> (-2.54%) ⬇️
core/system/rule_manager.go 79.03% <9.09%> (-15.20%) ⬇️
core/system/slot.go 54.90% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9ddf58...745ba60. Read the comment docs.

@louyuting louyuting requested review from sczyh30 and removed request for sczyh30 September 12, 2020 19:08
@louyuting louyuting added area/flow-control Issues or PRs related to flow control kind/enhancement Category issues or PRs related to enhancement to-review PRs to review and removed area/flow-control Issues or PRs related to flow control labels Sep 12, 2020
@louyuting louyuting changed the title Refine GetResRule func and GetRules func in flow/circuitbreaker/hotspot/system module Refine GetResRule API and GetRules API in flow/circuitbreaker/hotspot/system module Sep 12, 2020
// It doesn't take effect for circuit breaker module if user changes the rule.
// GetResRules need to compete circuit breaker module's global lock and the high performance losses of copy,
// reduce or do not call GetResRules frequently if possible
func GetResRules(resource string) []Rule {
Copy link
Member

Choose a reason for hiding this comment

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

How about GetRulesOfResource?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's ok for me.

Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit 5250310 into alibaba:master Sep 14, 2020
@sczyh30
Copy link
Member

sczyh30 commented Sep 14, 2020

Thanks!

@sczyh30 sczyh30 removed the to-review PRs to review label Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or PRs related to enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants