Skip to content

Commit

Permalink
Fixes bug in GCC implementation
Browse files Browse the repository at this point in the history
This fixes an issue described in #271.
The original implementation did not transition rate controller states
properly.
  • Loading branch information
sterlingdeng committed Dec 21, 2024
1 parent e187410 commit aecb15e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/gcc/rate_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ func (c *rateController) onDelayStats(ds DelayStats) {
return
}
c.delayStats = ds
c.delayStats.State = c.delayStats.State.transition(ds.Usage)
c.delayStats.State = c.lastState.transition(ds.Usage)
c.lastState = c.delayStats.State

if c.delayStats.State == stateHold {
return
Expand Down
55 changes: 55 additions & 0 deletions pkg/gcc/rate_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,58 @@ func TestRateControllerRun(t *testing.T) {
})
}
}

func TestRateController_StateTransition(t *testing.T) {
tcs := []struct {
name string
delayStats []DelayStats
wantStates []state
}{
{
name: "overuse-normal",
delayStats: []DelayStats{DelayStats{Usage: usageOver}, DelayStats{Usage: usageNormal}},
wantStates: []state{stateDecrease, stateHold},
},
{
name: "overuse-underuse",
delayStats: []DelayStats{DelayStats{Usage: usageOver}, DelayStats{Usage: usageUnder}},
wantStates: []state{stateDecrease, stateHold},
},
{
name: "normal",
delayStats: []DelayStats{DelayStats{Usage: usageNormal}},
wantStates: []state{stateIncrease},
},
{
name: "under-over",
delayStats: []DelayStats{DelayStats{Usage: usageUnder}, DelayStats{Usage: usageOver}},
wantStates: []state{stateHold, stateDecrease},
},
{
name: "under-normal",
delayStats: []DelayStats{DelayStats{Usage: usageUnder}, DelayStats{Usage: usageNormal}},
wantStates: []state{stateHold, stateIncrease},
},
{
name: "under-under",
delayStats: []DelayStats{DelayStats{Usage: usageUnder}, DelayStats{Usage: usageUnder}},
wantStates: []state{stateHold, stateHold},
},
}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
cb := func(ds DelayStats) {}

Check failure on line 120 in pkg/gcc/rate_controller_test.go

View workflow job for this annotation

GitHub Actions / lint / Go

unused-parameter: parameter 'ds' seems to be unused, consider removing or renaming it as _ (revive)
rc := newRateController(time.Now, 500_000, 100_000, 1_000_000, cb)
// Call it once to initialize the rate controller
rc.onDelayStats(DelayStats{})

for i, ds := range tc.delayStats {
rc.onDelayStats(ds)
if rc.lastState != tc.wantStates[i] {
t.Errorf("expected lastState to be %v but got %v", tc.wantStates[i], rc.lastState)
}
}
})
}
}

0 comments on commit aecb15e

Please sign in to comment.