fix bug in circuit_breaker.go about SlowRequestRatio

[JayiceZ]

Describe what this PR does / why we need it

when slowRequestRatio was set to 1.0d, requests will never be blocked. Because the currentRatio will never be bigger than 100%

Does this pull request fix one issue?

Describe how you did it

Special treatment for case when slowRequestRatio was set to 1.0d

Describe how to verify it

Special notes for reviews

[CLAassistant]

All committers have signed the CLA.

[binbin0325]

Maybe the same problem exists in errorRatioCircuitBreaker

[JayiceZ]

Maybe the same problem exists in errorRatioCircuitBreaker

Exactly. Thanks for your tip

[louyuting]

The Rule.Threshold means the max value that circuit breaker allowed. If the statistic ratio is great than Threshold, the circuit breaker will open.

From my perspective, setting Rule.Threshold as 1.0 doesn't make sense. May be we should check (Threshold==1.0) in

sentinel-golang/core/circuitbreaker/rule_manager.go

Line 347 in 51b28fd

func IsValid(r *Rule) error {

[JayiceZ]

The Rule.Threshold means the max value that circuit breaker allowed. If the statistic ratio is great than Threshold, the circuit breaker will open.

From my perspective, setting Rule.Threshold as 1.0 doesn't make sense. May be we should check (Threshold==1.0) in

sentinel-golang/core/circuitbreaker/rule_manager.go

Line 347 in 51b28fd

func IsValid(r *Rule) error {

You make a good point! But I think it would be better to support this, in case this feature is needed in some scenarios? What do you think?

[louyuting]

Maybe we need to change checking logic.
Using "slowRatio >= b.maxSlowRequestRatio"、”errorRatio >= b.errorRatioThreshold“ and ”errorCount >= b.errorCountThreshold“

It's easier to understand.

[louyuting]

The Rule.Threshold means the max value that circuit breaker allowed. If the statistic ratio is great than Threshold, the circuit breaker will open.
From my perspective, setting Rule.Threshold as 1.0 doesn't make sense. May be we should check (Threshold==1.0) in

sentinel-golang/core/circuitbreaker/rule_manager.go

Line 347 in 51b28fd

func IsValid(r *Rule) error {

You make a good point! But I think it would be better to support this, in case this feature is needed in some scenarios? What do you think?

Maybe we need improve the checking logic here. Refer to my comments

[codecov-io]

Codecov Report

Merging #315 into master will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #315   +/-   ##
=======================================
  Coverage   50.31%   50.31%           
=======================================
  Files          68       68           
  Lines        3705     3705           
=======================================
  Hits         1864     1864           
  Misses       1580     1580           
  Partials      261      261           

Impacted Files	Coverage Δ
core/circuitbreaker/circuit_breaker.go	68.15% <0.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 3926f99...b4c57d4. Read the comment docs.

[louyuting]

Maybe you do a wrong update. Below is my comments:

Maybe we need to change checking logic.
Using "slowRatio >= b.maxSlowRequestRatio"、”errorRatio >= b.errorRatioThreshold“ and ”errorCount >= b.errorCountThreshold“
It's easier to understand.

[JayiceZ]

Maybe you do a wrong update. Below is my comments:

Maybe we need to change checking logic.
Using "slowRatio >= b.maxSlowRequestRatio"、”errorRatio >= b.errorRatioThreshold“ and ”errorCount >= b.errorCountThreshold“
It's easier to understand.

Modified.Thanks for suggestions

[louyuting]

Nice work and looking forward more~

[JayiceZ]

Nice work and looking forward more~

appreciated for your help