Add cache mechanism for LoadRules in flow module (#268)

binbin0325 · louyuting · web-flow · commit 854b3f7c09bc · 2020-11-16T20:51:16.000+08:00
* Add cache mechanism for LoadRules in flow module 

Co-authored-by: louyuting &lt;1849491904@qq.com&gt;
diff --git a/core/flow/rule_manager.go b/core/flow/rule_manager.go
@@ -2,6 +2,7 @@ package flow
 
 import (
 	"fmt"
+	"reflect"
 	"sync"
 
 	"github.com/alibaba/sentinel-golang/core/base"
@@ -34,6 +35,7 @@ var (
 		readOnlyMetric:    base.NopReadStat(),
 		writeOnlyMetric:   base.NopWriteStat(),
 	}
+	currentRules = make([]*Rule, 0)
 )
 
 func init() {
@@ -165,12 +167,22 @@ func onRuleUpdate(rules []*Rule) (err error) {
 		}
 	}
 	tcMap = m
+	currentRules = rules
 	return nil
 }
 
 // LoadRules loads the given flow rules to the rule manager, while all previous rules will be replaced.
+// the first returned value indicates whether do real load operation, if the rules is the same with previous rules, return false
 func LoadRules(rules []*Rule) (bool, error) {
 	// TODO: rethink the design
+	tcMux.RLock()
+	isEqual := reflect.DeepEqual(currentRules, rules)
+	tcMux.RUnlock()
+	if isEqual {
+		logging.Info("[Flow] Load rules is the same with current rules, so ignore load operation.")
+		return false, nil
+	}
+
 	err := onRuleUpdate(rules)
 	return true, err
 }
diff --git a/core/flow/rule_manager_test.go b/core/flow/rule_manager_test.go
@@ -418,3 +418,27 @@ func Test_buildRulesOfRes(t *testing.T) {
 		assert.True(t, tcs[3].boundStat == stat4)
 	})
 }
+
+func TestLoadRules(t *testing.T) {
+	t.Run("loadSameRules", func(t *testing.T) {
+		_, err := LoadRules([]*Rule{
+			{
+				Resource:               "some-test",
+				Threshold:              10,
+				TokenCalculateStrategy: Direct,
+				ControlBehavior:        Reject,
+			},
+		})
+		assert.Nil(t, err)
+		ok, err := LoadRules([]*Rule{
+			{
+				Resource:               "some-test",
+				Threshold:              10,
+				TokenCalculateStrategy: Direct,
+				ControlBehavior:        Reject,
+			},
+		})
+		assert.Nil(t, err)
+		assert.False(t, ok)
+	})
+}
diff --git a/ext/datasource/helper.go b/ext/datasource/helper.go
@@ -51,8 +51,8 @@ func FlowRulesUpdater(data interface{}) error {
 			fmt.Sprintf("Fail to type assert data to []flow.Rule or []*flow.Rule, in fact, data: %+v", data),
 		)
 	}
-	succ, err := flow.LoadRules(rules)
-	if succ && err == nil {
+	_, err := flow.LoadRules(rules)
+	if err == nil {
 		return nil
 	}
 	return NewError(

Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ package flow`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"fmt"`
	`5`	`+ "reflect"`
`5`	`6`	`"sync"`
`6`	`7`
`7`	`8`	`"github.com/alibaba/sentinel-golang/core/base"`
`@@ -34,6 +35,7 @@ var (`
`34`	`35`	`readOnlyMetric: base.NopReadStat(),`
`35`	`36`	`writeOnlyMetric: base.NopWriteStat(),`
`36`	`37`	`}`
	`38`	`+ currentRules = make([]*Rule, 0)`
`37`	`39`	`)`
`38`	`40`
`39`	`41`	`func init() {`
`@@ -165,12 +167,22 @@ func onRuleUpdate(rules []*Rule) (err error) {`
`165`	`167`	`}`
`166`	`168`	`}`
`167`	`169`	`tcMap = m`
	`170`	`+ currentRules = rules`
`168`	`171`	`return nil`
`169`	`172`	`}`
`170`	`173`
`171`	`174`	`// LoadRules loads the given flow rules to the rule manager, while all previous rules will be replaced.`
	`175`	`+// the first returned value indicates whether do real load operation, if the rules is the same with previous rules, return false`
`172`	`176`	`func LoadRules(rules []*Rule) (bool, error) {`
`173`	`177`	`// TODO: rethink the design`
	`178`	`+ tcMux.RLock()`
	`179`	`+ isEqual := reflect.DeepEqual(currentRules, rules)`
	`180`	`+ tcMux.RUnlock()`
	`181`	`+ if isEqual {`
	`182`	`+ logging.Info("[Flow] Load rules is the same with current rules, so ignore load operation.")`
	`183`	`+ return false, nil`
	`184`	`+ }`
	`185`	`+`
`174`	`186`	`err := onRuleUpdate(rules)`
`175`	`187`	`return true, err`
`176`	`188`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,8 +51,8 @@ func FlowRulesUpdater(data interface{}) error {`
`51`	`51`	`fmt.Sprintf("Fail to type assert data to []flow.Rule or []*flow.Rule, in fact, data: %+v", data),`
`52`	`52`	`)`
`53`	`53`	`}`
`54`		`- succ, err := flow.LoadRules(rules)`
`55`		`- if succ && err == nil {`
	`54`	`+ _, err := flow.LoadRules(rules)`
	`55`	`+ if err == nil {`
`56`	`56`	`return nil`
`57`	`57`	`}`
`58`	`58`	`return NewError(`