Repository: alibaba/sentinel-golang
Pull request #305 (2020-10-25 07:43:37)

Allow flow rule MaxQueueingTimeMs to be 0
<!--  Thanks for submitting a pull request! Here are some tips for you:
1. Please make sure you have read and understood the contributing guidelines: https://github.com/alibaba/blob/master/CONTRIBUTING.md
2. Please make sure the PR has a corresponding issue.
-->

### Describe what this PR does / why we need it
Allow flow rule MaxQueueingTimeMs to be 0 even when the control behavior is `Throttling`.

### Does this pull request fix one issue?

<!--If that, add "Fixes #xxxx" below in the next line. For example, Fixes #15. Otherwise, add "NONE" -->

### Describe how you did it


### Describe how to verify it


### Special notes for reviews


Patch:
diff --git a/core/base/stat.go b/core/base/stat.go
index 79e0601..28a2c69 100644
--- a/core/base/stat.go
+++ b/core/base/stat.go
@@ -1,7 +1,7 @@
 package base
 
 import (
-	"github.com/pkg/errors"
+	"errors"
 )
 
 type TimePredicate func(uint64) bool
diff --git a/core/flow/rule.go b/core/flow/rule.go
index b739642..4ddfc1f 100644
--- a/core/flow/rule.go
+++ b/core/flow/rule.go
@@ -74,12 +74,15 @@ type Rule struct {
 	ControlBehavior        ControlBehavior        `json:"controlBehavior"`
 	// Threshold means the threshold during StatIntervalInMs
 	// If StatIntervalInMs is 1000(1 second), Threshold means QPS
-	Threshold         float64          `json:"threshold"`
-	RelationStrategy  RelationStrategy `json:"relationStrategy"`
-	RefResource       string           `json:"refResource"`
-	MaxQueueingTimeMs uint32           `json:"maxQueueingTimeMs"`
-	WarmUpPeriodSec   uint32           `json:"warmUpPeriodSec"`
-	WarmUpColdFactor  uint32           `json:"warmUpColdFactor"`
+	Threshold        float64          `json:"threshold"`
+	RelationStrategy RelationStrategy `json:"relationStrategy"`
+	RefResource      string           `json:"refResource"`
+	// MaxQueueingTimeMs only takes effect when ControlBehavior is Throttling.
+	// When MaxQueueingTimeMs is 0, it means Throttling only controls interval of requests,
+	// and requests exceeding the threshold will be rejected directly.
+	MaxQueueingTimeMs uint32 `json:"maxQueueingTimeMs"`
+	WarmUpPeriodSec   uint32 `json:"warmUpPeriodSec"`
+	WarmUpColdFactor  uint32 `json:"warmUpColdFactor"`
 	// StatIntervalInMs indicates the statistic interval and it's the optional setting for flow Rule.
 	// If user doesn't set StatIntervalInMs, that means using default metric statistic of resource.
 	// If the StatIntervalInMs user specifies can not reuse the global statistic of resource,
diff --git a/core/flow/rule_manager.go b/core/flow/rule_manager.go
index 445f96e..04199a9 100644
--- a/core/flow/rule_manager.go
+++ b/core/flow/rule_manager.go
@@ -470,9 +470,6 @@ func IsValidRule(rule *Rule) error {
 			return errors.New("WarmUpColdFactor must be great than 1")
 		}
 	}
-	if rule.ControlBehavior == Throttling && rule.MaxQueueingTimeMs == 0 {
-		return errors.New("MaxQueueingTimeMs can't be 0 when control behavior is Throttling")
-	}
 	if rule.StatIntervalInMs > 10*60*1000 {
 		logging.Info("StatIntervalInMs is great than 10 minutes, less than 10 minutes is recommended.")
 	}


Test patch:
diff --git a/core/flow/rule_manager_test.go b/core/flow/rule_manager_test.go
index b6090b4..d1d23a1 100644
--- a/core/flow/rule_manager_test.go
+++ b/core/flow/rule_manager_test.go
@@ -54,14 +54,18 @@ func TestIsValidFlowRule(t *testing.T) {
 	badRule1 := &Rule{Threshold: 1, Resource: ""}
 	badRule2 := &Rule{Threshold: -1.9, Resource: "test"}
 	badRule3 := &Rule{Threshold: 5, Resource: "test", TokenCalculateStrategy: WarmUp, ControlBehavior: Reject}
-	goodRule1 := &Rule{Threshold: 10, Resource: "test", TokenCalculateStrategy: WarmUp, ControlBehavior: Throttling, WarmUpPeriodSec: 10, MaxQueueingTimeMs: 10, StatIntervalInMs: 1000}
 	badRule4 := &Rule{Threshold: 5, Resource: "test", TokenCalculateStrategy: WarmUp, ControlBehavior: Reject, StatIntervalInMs: 6000000}
 
+	goodRule1 := &Rule{Threshold: 10, Resource: "test", TokenCalculateStrategy: WarmUp, ControlBehavior: Throttling, WarmUpPeriodSec: 10, MaxQueueingTimeMs: 10, StatIntervalInMs: 1000}
+	goodRule2 := &Rule{Threshold: 10, Resource: "test", TokenCalculateStrategy: WarmUp, ControlBehavior: Throttling, WarmUpPeriodSec: 10, MaxQueueingTimeMs: 0, StatIntervalInMs: 1000}
+
 	assert.Error(t, IsValidRule(badRule1))
 	assert.Error(t, IsValidRule(badRule2))
 	assert.Error(t, IsValidRule(badRule3))
-	assert.NoError(t, IsValidRule(goodRule1))
 	assert.Error(t, IsValidRule(badRule4))
+
+	assert.NoError(t, IsValidRule(goodRule1))
+	assert.NoError(t, IsValidRule(goodRule2))
 }
 
 func TestGetRules(t *testing.T) {