rate: the state of the limiter should not be changed when the requests failed
In the following cases, the reserveN is a no-op, and the state of
the limiter should not be changed:
1. n exceeds the Limiter's burst size
2. maxFutureReserve is less than the waitDuration
Fixes golang/go#52584
Change-Id: I5f38afa5da696bc10178a4bd0640d92062a8b009
GitHub-Last-Rev: e408718f070679209aca64bfbc18e4297777c311
GitHub-Pull-Request: golang/time#22
Reviewed-on: https://go-review.googlesource.com/c/time/+/406154
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Sameer Ajmani <sameer@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
diff --git a/rate/rate.go b/rate/rate.go
index 8f7c29f..f0e0cf3 100644
--- a/rate/rate.go
+++ b/rate/rate.go
@@ -83,7 +83,7 @@
// TokensAt returns the number of tokens available at time t.
func (lim *Limiter) TokensAt(t time.Time) float64 {
lim.mu.Lock()
- _, _, tokens := lim.advance(t) // does not mutute lim
+ _, tokens := lim.advance(t) // does not mutate lim
lim.mu.Unlock()
return tokens
}
@@ -183,7 +183,7 @@
return
}
// advance time to now
- t, _, tokens := r.lim.advance(t)
+ t, tokens := r.lim.advance(t)
// calculate new number of tokens
tokens += restoreTokens
if burst := float64(r.lim.burst); tokens > burst {
@@ -304,7 +304,7 @@
lim.mu.Lock()
defer lim.mu.Unlock()
- t, _, tokens := lim.advance(t)
+ t, tokens := lim.advance(t)
lim.last = t
lim.tokens = tokens
@@ -321,7 +321,7 @@
lim.mu.Lock()
defer lim.mu.Unlock()
- t, _, tokens := lim.advance(t)
+ t, tokens := lim.advance(t)
lim.last = t
lim.tokens = tokens
@@ -356,7 +356,7 @@
}
}
- t, last, tokens := lim.advance(t)
+ t, tokens := lim.advance(t)
// Calculate the remaining number of tokens resulting from the request.
tokens -= float64(n)
@@ -379,15 +379,11 @@
if ok {
r.tokens = n
r.timeToAct = t.Add(waitDuration)
- }
- // Update state
- if ok {
+ // Update state
lim.last = t
lim.tokens = tokens
lim.lastEvent = r.timeToAct
- } else {
- lim.last = last
}
return r
@@ -396,7 +392,7 @@
// advance calculates and returns an updated state for lim resulting from the passage of time.
// lim is not changed.
// advance requires that lim.mu is held.
-func (lim *Limiter) advance(t time.Time) (newT time.Time, newLast time.Time, newTokens float64) {
+func (lim *Limiter) advance(t time.Time) (newT time.Time, newTokens float64) {
last := lim.last
if t.Before(last) {
last = t
@@ -409,7 +405,7 @@
if burst := float64(lim.burst); tokens > burst {
tokens = burst
}
- return t, last, tokens
+ return t, tokens
}
// durationFromTokens is a unit conversion function from the number of tokens to the duration
diff --git a/rate/rate_test.go b/rate/rate_test.go
index d0d9149..a063e35 100644
--- a/rate/rate_test.go
+++ b/rate/rate_test.go
@@ -427,6 +427,12 @@
runReserve(t, lim, request{t1, 2, t1, true}) // start at t1
runReserve(t, lim, request{t0, 1, t1, true}) // should violate Limit,Burst
runReserve(t, lim, request{t2, 2, t3, true})
+ // burst size is 2, so n=3 always fails, and the state of lim should not be changed
+ runReserve(t, lim, request{t0, 3, time.Time{}, false})
+ runReserve(t, lim, request{t2, 1, t4, true})
+ // the maxReserve is not enough so it fails, and the state of lim should not be changed
+ runReserveMax(t, lim, request{t0, 2, time.Time{}, false}, d)
+ runReserve(t, lim, request{t2, 1, t5, true})
}
func TestReserveJumpBackCancel(t *testing.T) {
