Refine the semantics of onComplete: executed when a passed request finished (#215)

louyuting · web-flow · commit 750aefbd8aca · 2020-08-27T19:10:24.000+08:00
diff --git a/api/api_test.go b/api/api_test.go
@@ -125,5 +125,5 @@ func Test_entryWithArgsAndChainBlock(t *testing.T) {
 	rcs2.AssertNumberOfCalls(t, "Check", 0)
 	ssm.AssertNumberOfCalls(t, "OnEntryPassed", 0)
 	ssm.AssertNumberOfCalls(t, "OnEntryBlocked", 1)
-	ssm.AssertNumberOfCalls(t, "OnCompleted", 1)
+	ssm.AssertNumberOfCalls(t, "OnCompleted", 0)
 }
diff --git a/core/base/slot_chain.go b/core/base/slot_chain.go
@@ -185,6 +185,14 @@ func (sc *SlotChain) Entry(ctx *EntryContext) *TokenResult {
 }
 
 func (sc *SlotChain) exit(ctx *EntryContext) {
+	if ctx == nil || ctx.Entry() == nil {
+		logging.Errorf("nil ctx or nil associated entry")
+		return
+	}
+	// The OnCompleted is called only when entry passed
+	if ctx.IsBlocked() {
+		return
+	}
 	for _, s := range sc.stats {
 		s.OnCompleted(ctx)
 	}
diff --git a/core/base/slot_chain_test.go b/core/base/slot_chain_test.go
@@ -303,7 +303,7 @@ func TestSlotChain_Entry_Block(t *testing.T) {
 	dsm.AssertNumberOfCalls(t, "Check", 1)
 	ssm.AssertNumberOfCalls(t, "OnEntryPassed", 0)
 	ssm.AssertNumberOfCalls(t, "OnEntryBlocked", 1)
-	ssm.AssertNumberOfCalls(t, "OnCompleted", 1)
+	ssm.AssertNumberOfCalls(t, "OnCompleted", 0)
 }
 
 type badPrepareSlotMock struct {
diff --git a/core/circuitbreaker/stat_slot.go b/core/circuitbreaker/stat_slot.go
@@ -20,10 +20,6 @@ func (c *MetricStatSlot) OnEntryBlocked(_ *base.EntryContext, _ *base.BlockError
 }
 
 func (c *MetricStatSlot) OnCompleted(ctx *base.EntryContext) {
-	// The slot will ignore blocked requests.
-	if ctx.RuleCheckResult == nil || ctx.RuleCheckResult.IsBlocked() {
-		return
-	}
 	res := ctx.Resource.Name()
 	err := ctx.Err()
 	rt := ctx.Rt()
diff --git a/core/hotspot/concurrency_stat_slot.go b/core/hotspot/concurrency_stat_slot.go
@@ -34,9 +34,6 @@ func (c *ConcurrencyStatSlot) OnEntryBlocked(ctx *base.EntryContext, blockError
 }
 
 func (c *ConcurrencyStatSlot) OnCompleted(ctx *base.EntryContext) {
-	if ctx.IsBlocked() {
-		return
-	}
 	res := ctx.Resource.Name()
 	args := ctx.Input.Args
 	tcs := getTrafficControllersFor(res)
diff --git a/core/stat/stat_slot.go b/core/stat/stat_slot.go
@@ -29,9 +29,6 @@ func (s *StatisticSlot) OnEntryBlocked(ctx *base.EntryContext, blockError *base.
 }
 
 func (s *StatisticSlot) OnCompleted(ctx *base.EntryContext) {
-	if ctx.RuleCheckResult == nil || ctx.RuleCheckResult.IsBlocked() {
-		return
-	}
 	rt := util.CurrentTimeMillis() - ctx.StartTime()
 	ctx.PutRt(rt)
 	s.recordCompleteFor(ctx.StatNode, ctx.Input.AcquireCount, rt, ctx.Err())

Original file line number	Diff line number	Diff line change
`@@ -125,5 +125,5 @@ func Test_entryWithArgsAndChainBlock(t *testing.T) {`
`125`	`125`	`rcs2.AssertNumberOfCalls(t, "Check", 0)`
`126`	`126`	`ssm.AssertNumberOfCalls(t, "OnEntryPassed", 0)`
`127`	`127`	`ssm.AssertNumberOfCalls(t, "OnEntryBlocked", 1)`
`128`		`- ssm.AssertNumberOfCalls(t, "OnCompleted", 1)`
	`128`	`+ ssm.AssertNumberOfCalls(t, "OnCompleted", 0)`
`129`	`129`	`}`
Original file line number	Diff line number	Diff line change
`@@ -185,6 +185,14 @@ func (sc SlotChain) Entry(ctx EntryContext) *TokenResult {`
`185`	`185`	`}`
`186`	`186`
`187`	`187`	`func (sc SlotChain) exit(ctx EntryContext) {`
	`188`	`+ if ctx == nil \|\| ctx.Entry() == nil {`
	`189`	`+ logging.Errorf("nil ctx or nil associated entry")`
	`190`	`+ return`
	`191`	`+ }`
	`192`	`+ // The OnCompleted is called only when entry passed`
	`193`	`+ if ctx.IsBlocked() {`
	`194`	`+ return`
	`195`	`+ }`
`188`	`196`	`for _, s := range sc.stats {`
`189`	`197`	`s.OnCompleted(ctx)`
`190`	`198`	`}`
Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,7 @@ func TestSlotChain_Entry_Block(t *testing.T) {`
`303`	`303`	`dsm.AssertNumberOfCalls(t, "Check", 1)`
`304`	`304`	`ssm.AssertNumberOfCalls(t, "OnEntryPassed", 0)`
`305`	`305`	`ssm.AssertNumberOfCalls(t, "OnEntryBlocked", 1)`
`306`		`- ssm.AssertNumberOfCalls(t, "OnCompleted", 1)`
	`306`	`+ ssm.AssertNumberOfCalls(t, "OnCompleted", 0)`
`307`	`307`	`}`
`308`	`308`
`309`	`309`	`type badPrepareSlotMock struct {`
Original file line number	Diff line number	Diff line change
`@@ -34,9 +34,6 @@ func (c ConcurrencyStatSlot) OnEntryBlocked(ctx base.EntryContext, blockError`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`func (c ConcurrencyStatSlot) OnCompleted(ctx base.EntryContext) {`
`37`		`- if ctx.IsBlocked() {`
`38`		`- return`
`39`		`- }`
`40`	`37`	`res := ctx.Resource.Name()`
`41`	`38`	`args := ctx.Input.Args`
`42`	`39`	`tcs := getTrafficControllersFor(res)`
Original file line number	Diff line number	Diff line change
`@@ -29,9 +29,6 @@ func (s StatisticSlot) OnEntryBlocked(ctx base.EntryContext, blockError *base.`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`func (s StatisticSlot) OnCompleted(ctx base.EntryContext) {`
`32`		`- if ctx.RuleCheckResult == nil \|\| ctx.RuleCheckResult.IsBlocked() {`
`33`		`- return`
`34`		`- }`
`35`	`32`	`rt := util.CurrentTimeMillis() - ctx.StartTime()`
`36`	`33`	`ctx.PutRt(rt)`
`37`	`34`	`s.recordCompleteFor(ctx.StatNode, ctx.Input.AcquireCount, rt, ctx.Err())`