-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Refactor the underlying model of SystemRuleManager #3158
base: master
Are you sure you want to change the base?
Conversation
assertEquals(1.2d, rules.get(0).getHighestSystemLoad(), 0.01); | ||
assertEquals(1.2d, SystemRuleManager.getSystemLoadThreshold(), 0.01); | ||
assertEquals(1.2d, rules.get(0).getTriggerCount(), 0.01); | ||
assertEquals(1.2d, SystemRuleManager.getRules().get(0).getTriggerCount(), 0.01); | ||
} | ||
|
||
@Test | ||
public void testCheckMaxCpuUsageNotBBR() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failed, please fix it.
public int getType() { | ||
return type; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A formatting mistake here.
public void setMaxThread(long maxThread) { | ||
this.maxThread = maxThread; | ||
} | ||
private double triggerCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use count to name it like any other rule.
*/ | ||
public class SystemRuleManagerTest { | ||
|
||
@Test | ||
public void testLoadInvalidRules() { | ||
SystemRule rule1 = new SystemRule(); | ||
rule1.setHighestSystemLoad(-0.9d); | ||
rule1.setTriggerCount(-0.9d); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid this naming scheme, loadRule, cpuUsageRule may be more fitting.
@@ -87,19 +87,33 @@ public void run() { | |||
} | |||
|
|||
private static void initSystemRule() { | |||
SystemRule rule = new SystemRule(); | |||
SystemRule rule1 = new SystemRule(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
@@ -0,0 +1,41 @@ | |||
package com.alibaba.csp.sentinel.slots.system; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please prepend the license header here?
@@ -0,0 +1,116 @@ | |||
package com.alibaba.csp.sentinel.slots.system; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
Describe what this PR does / why we need it
重构 SystemRule, 参考了 sentinel-golang 对 SystemRule 的实现, 一个系统规则对应一个指标条件
Does this pull request fix one issue?
Fixes #3143 #3084 #3148
Describe how you did it
Describe how to verify it
运行单元测试
运行验证 demo
com.alibaba.csp.sentinel.demo.system.SystemGuardDemo#main()
Special notes for reviews