From 97ab31b5d95bdfcd1d646db0d87b0ec7a7ad6cb5 Mon Sep 17 00:00:00 2001
From: CoderSufiyan <sufiyanandkhan@gmail.com>
Date: Wed, 8 Jul 2026 20:17:39 +0530
Subject: [PATCH 1/2] fix: invert shouldRefill() condition in v1 scheduler rate
 limiter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The shouldRefill() method in the v1 scheduler rate limiter had an
inverted boolean condition. It returned true when nextRefillAt was
still in the future (window still open, no refill needed) and false
when nextRefillAt was in the past (window closed, refill needed).

This caused two issues:
1. Constant write-lock churn in use() — shouldRefill() returned true
   almost always (when window was open), triggering unnecessary
   flushToDatabase() calls that immediately no-opped.
2. Spurious rate-limit rejections after window rollover — shouldRefill()
   returned false, so use() never triggered the on-demand refresh.

Flipping the condition with '!' aligns shouldRefill() with the
flushToDatabase() guard, which correctly no-ops while nextRefillAt.After(now).

Fixes #4346
---
 pkg/scheduling/v1/rate_limit.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/scheduling/v1/rate_limit.go b/pkg/scheduling/v1/rate_limit.go
index 553c12102b..5227d02bda 100644
--- a/pkg/scheduling/v1/rate_limit.go
+++ b/pkg/scheduling/v1/rate_limit.go
@@ -179,7 +179,7 @@ func (r *rateLimiter) shouldRefill() bool {
 		return false
 	}
 
-	return r.nextRefillAt.After(time.Now().UTC())
+	return !r.nextRefillAt.After(time.Now().UTC())
 }
 
 func (r *rateLimiter) copyDbRateLimits() rateLimitSet {

From ae8515ad2e5d197799524d3eb4f34721d233630a Mon Sep 17 00:00:00 2001
From: CoderSufiyan <sufiyanandkhan@gmail.com>
Date: Fri, 10 Jul 2026 21:50:41 +0530
Subject: [PATCH 2/2] test: add unit tests for shouldRefill() and
 expired-window refresh

- TestRateLimiter_ShouldRefill covers nil/future/past nextRefillAt
- TestRateLimiter_UseRefreshesFromDbOnExpiredWindow verifies that
  use() triggers flushToDatabase when the refill window has passed
  and a depleted rate limit is refreshed from the database
---
 pkg/scheduling/v1/rate_limit_test.go | 66 ++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/pkg/scheduling/v1/rate_limit_test.go b/pkg/scheduling/v1/rate_limit_test.go
index b3a52b0193..c1ed00a332 100644
--- a/pkg/scheduling/v1/rate_limit_test.go
+++ b/pkg/scheduling/v1/rate_limit_test.go
@@ -218,6 +218,72 @@ func TestRateLimiter_FlushToDatabase(t *testing.T) {
 	assert.Empty(t, rateLimiter.unflushed)
 }
 
+func TestRateLimiter_ShouldRefill(t *testing.T) {
+	l := zerolog.Nop()
+
+	t.Run("nil nextRefillAt returns false", func(t *testing.T) {
+		r := &rateLimiter{
+			dbRateLimits: make(rateLimitSet),
+			unacked:      make(map[int64]rateLimitSet),
+			unflushed:    make(rateLimitSet),
+			l:            &l,
+		}
+		assert.False(t, r.shouldRefill())
+	})
+
+	t.Run("future nextRefillAt returns false", func(t *testing.T) {
+		future := time.Now().Add(time.Hour)
+		r := &rateLimiter{
+			nextRefillAt: &future,
+			dbRateLimits: make(rateLimitSet),
+			unacked:      make(map[int64]rateLimitSet),
+			unflushed:    make(rateLimitSet),
+			l:            &l,
+		}
+		assert.False(t, r.shouldRefill())
+	})
+
+	t.Run("past nextRefillAt returns true", func(t *testing.T) {
+		past := time.Now().Add(-time.Hour)
+		r := &rateLimiter{
+			nextRefillAt: &past,
+			dbRateLimits: make(rateLimitSet),
+			unacked:      make(map[int64]rateLimitSet),
+			unflushed:    make(rateLimitSet),
+			l:            &l,
+		}
+		assert.True(t, r.shouldRefill())
+	})
+}
+
+func TestRateLimiter_UseRefreshesFromDbOnExpiredWindow(t *testing.T) {
+	l := zerolog.Nop()
+
+	mockRateLimitRepo := &mockRateLimitRepo{}
+	mockRows := []*sqlcv1.ListRateLimitsForTenantWithMutateRow{
+		{Key: "key1", Value: 10},
+	}
+	past := time.Now().Add(-time.Second)
+	mockRateLimitRepo.On("UpdateRateLimits", context.Background(), mock.Anything, mock.Anything).Return(mockRows, &past, nil)
+
+	rateLimiter := &rateLimiter{
+		dbRateLimits: rateLimitSet{
+			"key1": {key: "key1", val: 0, nextRefillAt: &past},
+		},
+		nextRefillAt:   &past,
+		unacked:        make(map[int64]rateLimitSet),
+		unflushed:      make(rateLimitSet),
+		l:              &l,
+		rateLimitRepo:  mockRateLimitRepo,
+	}
+
+	// use() detects the expired window via shouldRefill(), triggers flushToDatabase,
+	// and the refreshed limits (val: 10) should allow the request
+	res := rateLimiter.use(context.Background(), 1, map[string]int32{"key1": 5})
+	assert.True(t, res.succeeded)
+	mockRateLimitRepo.AssertExpectations(t)
+}
+
 func BenchmarkRateLimiter(b *testing.B) {
 	l := zerolog.Nop()