Skip to content

Commit 577d59e

Browse files
author
sterlingdeng
committed
Fixes bug in GCC implementation
This fixes an issue described in pion#271. The original implementation did not transition rate controller states properly.
1 parent e187410 commit 577d59e

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

pkg/gcc/rate_controller.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ func (c *rateController) onDelayStats(ds DelayStats) {
9090
return
9191
}
9292
c.delayStats = ds
93-
c.delayStats.State = c.delayStats.State.transition(ds.Usage)
93+
c.delayStats.State = c.lastState.transition(ds.Usage)
94+
c.lastState = c.delayStats.State
9495

9596
if c.delayStats.State == stateHold {
9697
return

pkg/gcc/rate_controller_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,57 @@ func TestRateControllerRun(t *testing.T) {
7676
})
7777
}
7878
}
79+
80+
func TestRateController_StateTransition(t *testing.T) {
81+
tcs := []struct {
82+
name string
83+
delayStats []DelayStats
84+
wantStates []state
85+
}{
86+
{
87+
name: "overuse-normal",
88+
delayStats: []DelayStats{{Usage: usageOver}, {Usage: usageNormal}},
89+
wantStates: []state{stateDecrease, stateHold},
90+
},
91+
{
92+
name: "overuse-underuse",
93+
delayStats: []DelayStats{{Usage: usageOver}, {Usage: usageUnder}},
94+
wantStates: []state{stateDecrease, stateHold},
95+
},
96+
{
97+
name: "normal",
98+
delayStats: []DelayStats{{Usage: usageNormal}},
99+
wantStates: []state{stateIncrease},
100+
},
101+
{
102+
name: "under-over",
103+
delayStats: []DelayStats{{Usage: usageUnder}, {Usage: usageOver}},
104+
wantStates: []state{stateHold, stateDecrease},
105+
},
106+
{
107+
name: "under-normal",
108+
delayStats: []DelayStats{{Usage: usageUnder}, {Usage: usageNormal}},
109+
wantStates: []state{stateHold, stateIncrease},
110+
},
111+
{
112+
name: "under-under",
113+
delayStats: []DelayStats{{Usage: usageUnder}, {Usage: usageUnder}},
114+
wantStates: []state{stateHold, stateHold},
115+
},
116+
}
117+
118+
for _, tc := range tcs {
119+
t.Run(tc.name, func(t *testing.T) {
120+
rc := newRateController(time.Now, 500_000, 100_000, 1_000_000, func(DelayStats) {})
121+
// Call it once to initialize the rate controller
122+
rc.onDelayStats(DelayStats{})
123+
124+
for i, ds := range tc.delayStats {
125+
rc.onDelayStats(ds)
126+
if rc.lastState != tc.wantStates[i] {
127+
t.Errorf("expected lastState to be %v but got %v", tc.wantStates[i], rc.lastState)
128+
}
129+
}
130+
})
131+
}
132+
}

0 commit comments

Comments
 (0)