Refine APIs of per-resource slot chain and improve rule slot registering logic in rule managers (#291)

* refine api and comments, and add unit tests

Author: louyuting

api/api.go

@@ -130,7 +130,7 @@ func Entry(resource string, opts ...EntryOption) (*base.SentinelEntry, *base.Blo
 	if options.slotChain == nil {
 		options.slotChain = misc.GetResourceSlotChain(resource)
 		if options.slotChain == nil {
-			options.slotChain = globalSlotChain
+			options.slotChain = GlobalSlotChain()
 		}
 	}
 	return entry(resource, options)

api/slot_chain.go

@@ -33,7 +33,23 @@ func BuildDefaultSlotChain() *base.SlotChain {
 	sc.AddStatSlotLast(circuitbreaker.DefaultMetricStatSlot)
 	sc.AddStatSlotLast(hotspot.DefaultConcurrencyStatSlot)
 	sc.AddStatSlotLast(flow.DefaultStandaloneStatSlot)
-
-	misc.RegisterCustomGlobalSlotsToSc(sc)
 	return sc
 }
+
+// RegisterGlobalStatPrepareSlot registers the global StatPrepareSlot for all resource
+// Note: this function is not thread-safe
+func RegisterGlobalStatPrepareSlot(slot base.StatPrepareSlot) {
+	misc.RegisterGlobalStatPrepareSlot(slot)
+}
+
+// RegisterGlobalRuleCheckSlot registers the global RuleCheckSlot for all resource
+// Note: this function is not thread-safe
+func RegisterGlobalRuleCheckSlot(slot base.RuleCheckSlot) {
+	misc.RegisterGlobalRuleCheckSlot(slot)
+}
+
+// RegisterGlobalStatSlot registers the global StatSlot for all resource
+// Note: this function is not thread-safe
+func RegisterGlobalStatSlot(slot base.StatSlot) {
+	misc.RegisterGlobalStatSlot(slot)
+}

core/base/slot_chain.go

@@ -83,9 +83,9 @@ var (
 
 func NewSlotChain() *SlotChain {
 	return &SlotChain{
-		statPres:   make([]StatPrepareSlot, 0, 5),
-		ruleChecks: make([]RuleCheckSlot, 0, 5),
-		stats:      make([]StatSlot, 0, 5),
+		statPres:   make([]StatPrepareSlot, 0, 8),
+		ruleChecks: make([]RuleCheckSlot, 0, 8),
+		stats:      make([]StatSlot, 0, 8),
 		ctxPool:    ctxPool,
 	}
 }
@@ -104,6 +104,21 @@ func (sc *SlotChain) RefurbishContext(c *EntryContext) {
 	}
 }
 
+// ValidateStatPrepareSlotNaming checks whether the name of StatPrepareSlot exists in SlotChain.[]StatPrepareSlot
+// return true the name of StatPrepareSlot doesn't exist in SlotChain.[]StatPrepareSlot
+func ValidateStatPrepareSlotNaming(sc *SlotChain, s StatPrepareSlot) bool {
+	isValid := true
+	f := func(slot StatPrepareSlot) {
+		if slot.Name() == s.Name() {
+			isValid = false
+		}
+	}
+	sc.RangeStatPrepareSlot(f)
+
+	return isValid
+}
+
+// RangeStatPrepareSlot iterates the SlotChain.[]StatPrepareSlot and call f function for each StatPrepareSlot
 func (sc *SlotChain) RangeStatPrepareSlot(f func(slot StatPrepareSlot)) {
 	for _, slot := range sc.statPres {
 		f(slot)
@@ -121,6 +136,21 @@ func (sc *SlotChain) AddStatPrepareSlotLast(s StatPrepareSlot) {
 	sc.statPres = append(sc.statPres, s)
 }
 
+// ValidateRuleCheckSlotNaming checks whether the name of RuleCheckSlot exists in SlotChain.[]RuleCheckSlot
+// return true the name of RuleCheckSlot doesn't exist in SlotChain.[]RuleCheckSlot
+func ValidateRuleCheckSlotNaming(sc *SlotChain, s RuleCheckSlot) bool {
+	isValid := true
+	f := func(slot RuleCheckSlot) {
+		if slot.Name() == s.Name() {
+			isValid = false
+		}
+	}
+	sc.RangeRuleCheckSlot(f)
+
+	return isValid
+}
+
+// RangeRuleCheckSlot iterates the SlotChain.[]RuleCheckSlot and call f function for each RuleCheckSlot
 func (sc *SlotChain) RangeRuleCheckSlot(f func(slot RuleCheckSlot)) {
 	for _, slot := range sc.ruleChecks {
 		f(slot)
@@ -137,6 +167,21 @@ func (sc *SlotChain) AddRuleCheckSlotLast(s RuleCheckSlot) {
 	sc.ruleChecks = append(sc.ruleChecks, s)
 }
 
+// ValidateStatSlotNaming checks whether the name of StatSlot exists in SlotChain.[]StatSlot
+// return true the name of StatSlot doesn't exist in SlotChain.[]StatSlot
+func ValidateStatSlotNaming(sc *SlotChain, s StatSlot) bool {
+	isValid := true
+	f := func(slot StatSlot) {
+		if slot.Name() == s.Name() {
+			isValid = false
+		}
+	}
+	sc.RangeStatSlot(f)
+
+	return isValid
+}
+
+// RangeStatSlot iterates the SlotChain.[]StatSlot and call f function for each StatSlot
 func (sc *SlotChain) RangeStatSlot(f func(slot StatSlot)) {
 	for _, slot := range sc.stats {
 		f(slot)

core/base/slot_chain_test.go

@@ -1,7 +1,6 @@
 package base
 
 import (
-	"fmt"
 	"reflect"
 	"strconv"
 	"testing"
@@ -16,11 +15,10 @@ type StatPrepareSlotMock1 struct {
 }
 
 func (spl *StatPrepareSlotMock1) Name() string {
-	return "mock-sentinel-prepare-slot1"
+	return spl.name
 }
 
 func (spl *StatPrepareSlotMock1) Prepare(ctx *EntryContext) {
-	fmt.Println(spl.name)
 	return
 }
 
@@ -72,11 +70,10 @@ type RuleCheckSlotMock1 struct {
 }
 
 func (rcs *RuleCheckSlotMock1) Name() string {
-	return "mock-sentinel-rule-check-slot-1"
+	return rcs.name
 }
 
 func (rcs *RuleCheckSlotMock1) Check(ctx *EntryContext) *TokenResult {
-	fmt.Println(rcs.name)
 	return nil
 }
 func TestSlotChain_addRuleCheckSlotFirstAndLast(t *testing.T) {
@@ -127,17 +124,17 @@ type StatSlotMock1 struct {
 }
 
 func (ss *StatSlotMock1) Name() string {
-	return "mock-sentinel-stat-mock-slot-1"
+	return ss.name
 }
 
 func (ss *StatSlotMock1) OnEntryPassed(ctx *EntryContext) {
-	fmt.Println(ss.name)
+	return
 }
 func (ss *StatSlotMock1) OnEntryBlocked(ctx *EntryContext, blockError *BlockError) {
-	fmt.Printf("%s blocked: %v\n", ss.name, blockError)
+	return
 }
 func (ss *StatSlotMock1) OnCompleted(ctx *EntryContext) {
-	fmt.Println(ss.name)
+	return
 }
 func TestSlotChain_addStatSlotFirstAndLast(t *testing.T) {
 	sc := NewSlotChain()
@@ -387,3 +384,90 @@ func TestSlotChain_Entry_With_Panic(t *testing.T) {
 	ssm.AssertNumberOfCalls(t, "OnEntryPassed", 0)
 	ssm.AssertNumberOfCalls(t, "OnEntryBlocked", 0)
 }
+
+func TestValidateStatPrepareSlotNaming(t *testing.T) {
+	sc := NewSlotChain()
+	sps1 := &StatPrepareSlotMock1{
+		name: "sps1",
+	}
+	sps2 := &StatPrepareSlotMock1{
+		name: "sps2",
+	}
+	sps3 := &StatPrepareSlotMock1{
+		name: "sps3",
+	}
+	sps4 := &StatPrepareSlotMock1{
+		name: "sps4",
+	}
+	sc.AddStatPrepareSlotLast(sps1)
+	sc.AddStatPrepareSlotLast(sps2)
+	sc.AddStatPrepareSlotLast(sps3)
+	sc.AddStatPrepareSlotLast(sps4)
+
+	sps5 := &StatPrepareSlotMock1{
+		name: "sps5",
+	}
+	assert.True(t, ValidateStatPrepareSlotNaming(sc, sps5))
+	sps6 := &StatPrepareSlotMock1{
+		name: "sps1",
+	}
+	assert.True(t, !ValidateStatPrepareSlotNaming(sc, sps6))
+}
+
+func TestValidateRuleCheckSlotNaming(t *testing.T) {
+	sc := NewSlotChain()
+	rcs1 := &RuleCheckSlotMock1{
+		name: "rcs1",
+	}
+	rcs2 := &RuleCheckSlotMock1{
+		name: "rcs2",
+	}
+	rcs3 := &RuleCheckSlotMock1{
+		name: "rcs3",
+	}
+	rcs4 := &RuleCheckSlotMock1{
+		name: "rcs4",
+	}
+	sc.AddRuleCheckSlotLast(rcs1)
+	sc.AddRuleCheckSlotLast(rcs2)
+	sc.AddRuleCheckSlotLast(rcs3)
+	sc.AddRuleCheckSlotLast(rcs4)
+
+	rcs5 := &RuleCheckSlotMock1{
+		name: "rcs5",
+	}
+	assert.True(t, ValidateRuleCheckSlotNaming(sc, rcs5))
+	rcs6 := &RuleCheckSlotMock1{
+		name: "rcs1",
+	}
+	assert.True(t, !ValidateRuleCheckSlotNaming(sc, rcs6))
+}
+
+func TestValidateStatSlotNaming(t *testing.T) {
+	sc := NewSlotChain()
+	ss1 := &StatSlotMock1{
+		name: "ss1",
+	}
+	ss2 := &StatSlotMock1{
+		name: "ss2",
+	}
+	ss3 := &StatSlotMock1{
+		name: "ss3",
+	}
+	ss4 := &StatSlotMock1{
+		name: "ss4",
+	}
+	sc.AddStatSlotLast(ss1)
+	sc.AddStatSlotLast(ss2)
+	sc.AddStatSlotLast(ss3)
+	sc.AddStatSlotLast(ss4)
+
+	ss5 := &StatSlotMock1{
+		name: "ss5",
+	}
+	assert.True(t, ValidateStatSlotNaming(sc, ss5))
+	ss6 := &StatSlotMock1{
+		name: "ss1",
+	}
+	assert.True(t, !ValidateStatSlotNaming(sc, ss6))
+}

core/circuitbreaker/rule_manager.go

@@ -198,10 +198,6 @@ func onRuleUpdate(rules []*Rule) (err error) {
 		}
 		ruleSet = append(ruleSet, rule)
 		newBreakerRules[classification] = ruleSet
-
-		// update resource slot chain
-		misc.RegisterResourceRuleCheckSlot(rule.Resource, DefaultSlot)
-		misc.RegisterResourceStatSlot(rule.Resource, DefaultMetricStatSlot)
 	}
 
 	newBreakers := make(map[string][]CircuitBreaker)
@@ -265,6 +261,14 @@ func onRuleUpdate(rules []*Rule) (err error) {
 		}
 	}
 
+	for res, cbs := range newBreakers {
+		if len(cbs) > 0 {
+			// update resource slot chain
+			misc.RegisterRuleCheckSlotForResource(res, DefaultSlot)
+			misc.RegisterStatSlotForResource(res, DefaultMetricStatSlot)
+		}
+	}
+
 	breakerRules = newBreakerRules
 	breakers = newBreakers
 	return nil

core/flow/rule_manager.go

@@ -142,10 +142,6 @@ func onRuleUpdate(rules []*Rule) (err error) {
 			resRules = make([]*Rule, 0, 1)
 		}
 		resRulesMap[rule.Resource] = append(resRules, rule)
-
-		// update resource slot chain
-		misc.RegisterResourceRuleCheckSlot(rule.Resource, DefaultSlot)
-		misc.RegisterResourceStatSlot(rule.Resource, DefaultStandaloneStatSlot)
 	}
 	m := make(TrafficControllerMap, len(resRulesMap))
 	start := util.CurrentTimeNano()
@@ -161,6 +157,14 @@ func onRuleUpdate(rules []*Rule) (err error) {
 	for res, rulesOfRes := range resRulesMap {
 		m[res] = buildRulesOfRes(res, rulesOfRes)
 	}
+
+	for res, tcs := range m {
+		if len(tcs) > 0 {
+			// update resource slot chain
+			misc.RegisterRuleCheckSlotForResource(res, DefaultSlot)
+			misc.RegisterStatSlotForResource(res, DefaultStandaloneStatSlot)
+		}
+	}
 	tcMap = m
 	return nil
 }

core/hotspot/rule_manager.go

@@ -130,10 +130,6 @@ func onRuleUpdate(rules []*Rule) (err error) {
 		}
 		ruleSet = append(ruleSet, r)
 		newRuleMap[res] = ruleSet
-
-		// update resource slot chain
-		misc.RegisterResourceRuleCheckSlot(r.Resource, DefaultSlot)
-		misc.RegisterResourceStatSlot(r.Resource, DefaultConcurrencyStatSlot)
 	}
 
 	m := make(trafficControllerMap)
@@ -195,6 +191,13 @@ func onRuleUpdate(rules []*Rule) (err error) {
 			insertTcToTcMap(tc, res, m)
 		}
 	}
+	for res, tcs := range m {
+		if len(tcs) > 0 {
+			// update resource slot chain
+			misc.RegisterRuleCheckSlotForResource(res, DefaultSlot)
+			misc.RegisterStatSlotForResource(res, DefaultConcurrencyStatSlot)
+		}
+	}
 	tcMap = m
 
 	return nil

core/isolation/rule_manager.go

@@ -30,9 +30,6 @@ func LoadRules(rules []*Rule) (updated bool, err error) {
 			resRules = make([]*Rule, 0, 1)
 		}
 		m[r.Resource] = append(resRules, r)
-
-		// update resource slot chain
-		misc.RegisterResourceRuleCheckSlot(r.Resource, DefaultSlot)
 	}
 
 	start := util.CurrentTimeNano()
@@ -42,6 +39,13 @@ func LoadRules(rules []*Rule) (updated bool, err error) {
 		logging.Debug("time statistic(ns) for updating isolation rule", "timeCost", util.CurrentTimeNano()-start)
 		logRuleUpdate(m)
 	}()
+
+	for res, rs := range m {
+		if len(rs) > 0 {
+			// update resource slot chain
+			misc.RegisterRuleCheckSlotForResource(res, DefaultSlot)
+		}
+	}
 	ruleMap = m
 	return
 }