amadan
edge: check the door bucket before touching the visitor LRU; refill() dedupe
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+46
−14
internal/edge/doorlimit.go+14 −13internal/edge/doorlimit_test.go+32 −1
diff --git a/internal/edge/doorlimit.go b/internal/edge/doorlimit.go| index 8fe0a1621..cfb7f9644 100644 |
| --- a/internal/edge/doorlimit.go |
| +++ b/internal/edge/doorlimit.go |
| @@ -22,9 +22,11 @@ type tokenBucket struct { |
| last time.Time |
| } |
| -func (b *tokenBucket) take(now time.Time) bool { |
| +// refill tops the bucket up for elapsed time since last, capped at cap. |
| +func (b *tokenBucket) refill(now time.Time) { |
| if b.last.IsZero() { |
| b.tokens, b.last = b.cap, now |
| + return |
| } |
| if el := now.Sub(b.last); el > 0 { |
| b.tokens += el.Seconds() * b.cap / 60 |
| @@ -33,6 +35,10 @@ func (b *tokenBucket) take(now time.Time) bool { |
| } |
| b.last = now |
| } |
| +} |
| + |
| +func (b *tokenBucket) take(now time.Time) bool { |
| + b.refill(now) |
| if b.tokens < 1 { |
| return false |
| } |
| @@ -41,16 +47,7 @@ func (b *tokenBucket) take(now time.Time) bool { |
| } |
| func (b *tokenBucket) has(now time.Time) bool { // peek: refill, but spend nothing |
| - if b.last.IsZero() { |
| - b.tokens, b.last = b.cap, now |
| - } |
| - if el := now.Sub(b.last); el > 0 { |
| - b.tokens += el.Seconds() * b.cap / 60 |
| - if b.tokens > b.cap { |
| - b.tokens = b.cap |
| - } |
| - b.last = now |
| - } |
| + b.refill(now) |
| return b.tokens >= 1 |
| } |
| @@ -88,7 +85,11 @@ func (l *doorLimiter) allow(app string, door byte, visitor uint64, now time.Time |
| l.apps[app] = st |
| } |
| ab := st.bucket[door] |
| - if ab == nil { |
| + if ab == nil || !ab.has(now) { |
| + // Refused on the app bucket alone: touch neither the visitor |
| + // LRU nor its index, so a storm of never-seen visitors can't |
| + // evict legitimate visitor state while every post is dropped |
| + // anyway. |
| return false |
| } |
| el := st.visitorIdx[visitor] |
| @@ -104,7 +105,7 @@ func (l *doorLimiter) allow(app string, door byte, visitor uint64, now time.Time |
| st.visitors.MoveToFront(el) |
| } |
| vb := &el.Value.(*visitorEntry).b |
| - if !ab.has(now) || !vb.has(now) { |
| + if !vb.has(now) { |
| return false |
| } |
| return ab.take(now) && vb.take(now) |
diff --git a/internal/edge/doorlimit_test.go b/internal/edge/doorlimit_test.go| index 782f6e492..97d777176 100644 |
| --- a/internal/edge/doorlimit_test.go |
| +++ b/internal/edge/doorlimit_test.go |
| @@ -45,8 +45,14 @@ func TestDoorLimiterVisitorBucketAndLRU(t *testing.T) { |
| if got := l.apps["a/web"].bucket['a'].tokens; got < float64(doorAppPerMinute-61) { |
| t.Fatalf("app tokens %v, refused visitor posts must not spend them", got) |
| } |
| + // Space these out well past the app door's own refill window (60s) |
| + // so each post is admitted on the app bucket and only the visitor |
| + // LRU's own cap is under test here, not door 'a''s 600/minute. |
| for i := uint64(100); i < 100+doorVisitorEntries; i++ { |
| - l.allow("a/web", 'a', i, now) |
| + step := now.Add(time.Duration(i) * 61 * time.Second) |
| + if !l.allow("a/web", 'a', i, step) { |
| + t.Fatalf("visitor %d refused: app bucket should be fully refilled by then", i) |
| + } |
| } |
| if n := l.apps["a/web"].visitors.Len(); n != doorVisitorEntries { |
| t.Fatalf("visitor table %d entries, want the cap", n) |
| @@ -55,3 +61,28 @@ func TestDoorLimiterVisitorBucketAndLRU(t *testing.T) { |
| t.Fatal("the oldest visitor must have been evicted") |
| } |
| } |
| + |
| +func TestDoorLimiterAppBucketExhaustedSkipsVisitorLRU(t *testing.T) { |
| + l := newDoorLimiter() |
| + now := time.Unix(1_700_000_000, 0) |
| + for i := 0; i < doorAppPerMinute; i++ { |
| + if !l.allow("a/web", 'e', uint64(i), now) { |
| + t.Fatalf("post %d refused", i+1) |
| + } |
| + } |
| + st := l.apps["a/web"] |
| + before := st.visitors.Len() |
| + for i := uint64(1_000_000); i < 1_000_050; i++ { |
| + if l.allow("a/web", 'e', i, now) { |
| + t.Fatalf("visitor %d should be refused: the app bucket is empty", i) |
| + } |
| + } |
| + if got := st.visitors.Len(); got != before { |
| + t.Fatalf("visitor table grew from %d to %d: a refusal on the app bucket must not touch the LRU", before, got) |
| + } |
| + for i := uint64(1_000_000); i < 1_000_050; i++ { |
| + if _, ok := st.visitorIdx[i]; ok { |
| + t.Fatalf("visitor %d must not have been inserted: the app bucket was already empty", i) |
| + } |
| + } |
| +} |