Skip to content

Commit f4080ef

Browse files
jopemachineclaude
andcommitted
feat(BA-7365): key the rate limit counter by user id
`execute_rate_limit_logic()` and `get_rolling_count()` take a `UserID` and build the counter key themselves, and the manager's rate limit middleware counts against the authenticated user instead of the access key, so holding multiple keypairs no longer multiplies a user's allowance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 86911bb commit f4080ef

5 files changed

Lines changed: 34 additions & 26 deletions

File tree

src/ai/backend/common/clients/valkey_client/valkey_rate_limit/client.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
create_valkey_client,
1111
)
1212
from ai.backend.common.exception import BackendAIError
13+
from ai.backend.common.identifier.user import UserID
1314
from ai.backend.common.metrics.metric import DomainType, LayerType
1415
from ai.backend.common.resilience import (
1516
BackoffStrategy,
@@ -44,19 +45,19 @@
4445

4546

4647
_RATE_LIMIT_SCRIPT: Final[str] = """
47-
local access_key = KEYS[1]
48+
local key = KEYS[1]
4849
local now = tonumber(ARGV[1])
4950
local window = tonumber(ARGV[2])
5051
local request_id = tonumber(redis.call('INCR', '__request_id'))
5152
if request_id >= 1e12 then
5253
redis.call('SET', '__request_id', 1)
5354
end
54-
if redis.call('EXISTS', access_key) == 1 then
55-
redis.call('ZREMRANGEBYSCORE', access_key, 0, now - window)
55+
if redis.call('EXISTS', key) == 1 then
56+
redis.call('ZREMRANGEBYSCORE', key, 0, now - window)
5657
end
57-
redis.call('ZADD', access_key, now, tostring(request_id))
58-
redis.call('EXPIRE', access_key, window)
59-
return redis.call('ZCARD', access_key)
58+
redis.call('ZADD', key, now, tostring(request_id))
59+
redis.call('EXPIRE', key, window)
60+
return redis.call('ZCARD', key)
6061
"""
6162

6263

@@ -110,14 +111,14 @@ async def close(self) -> None:
110111
@valkey_rate_limit_resilience.apply()
111112
async def execute_rate_limit_logic(
112113
self,
113-
access_key: str,
114+
user_id: UserID,
114115
window: int = _DEFAULT_RATE_LIMIT_EXPIRATION,
115116
) -> int:
116117
"""
117118
Execute the rate limiting logic for rolling counter.
118119
This replicates the Lua script logic using individual commands.
119120
120-
:param access_key: The access key to rate limit.
121+
:param user_id: The user the rolling counter is keyed by.
121122
:param window: The time window for rate limiting in seconds.
122123
:return: The current count.
123124
"""
@@ -127,23 +128,23 @@ async def execute_rate_limit_logic(
127128
async with self._client.client() as conn:
128129
result = await conn.invoke_script(
129130
Script(_RATE_LIMIT_SCRIPT),
130-
keys=[access_key],
131+
keys=[f"user.{user_id}"],
131132
args=[str(now_float), str(window)],
132133
)
133134

134135
# The last result is the count
135136
return cast(int, result)
136137

137138
@valkey_rate_limit_resilience.apply()
138-
async def get_rolling_count(self, access_key: str) -> int:
139+
async def get_rolling_count(self, user_id: UserID) -> int:
139140
"""
140-
Get the current rolling count for an access key.
141+
Get the current rolling count of a user.
141142
142-
:param access_key: The access key to get the count for.
143+
:param user_id: The user the rolling counter is keyed by.
143144
:return: The current count.
144145
"""
145146
async with self._client.client() as conn:
146-
return await conn.zcard(access_key)
147+
return await conn.zcard(f"user.{user_id}")
147148

148149
@valkey_rate_limit_resilience.apply()
149150
async def set_rate_limit_config(

src/ai/backend/manager/api/gql_legacy/keypair.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
1515
from ai.backend.common.defs import REDIS_RATE_LIMIT_DB, RedisRole
16+
from ai.backend.common.identifier.user import UserID
1617
from ai.backend.common.types import AccessKey
1718
from ai.backend.manager.data.kernel.types import KernelStatus
1819
from ai.backend.manager.data.keypair.types import KeyPairCreator, KeyPairData
@@ -196,7 +197,7 @@ async def resolve_rolling_count(self, info: graphene.ResolveInfo) -> int:
196197
human_readable_name="ratelimit",
197198
)
198199
try:
199-
return await valkey_client.get_rolling_count(self.access_key)
200+
return await valkey_client.get_rolling_count(UserID(self.user))
200201
finally:
201202
await valkey_client.close()
202203

src/ai/backend/manager/api/rest/ratelimit/handler.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ async def rlim_middleware(
3838
"""Global middleware implementing a rolling-counter rate limiter."""
3939
if request["is_authorized"]:
4040
rate_limit = request["keypair"]["rate_limit"]
41-
access_key = request["keypair"]["access_key"]
4241
rolling_count = await valkey_client.execute_rate_limit_logic(
43-
access_key=access_key,
42+
user_id=request["user"]["uuid"],
4443
window=_rlim_window,
4544
)
4645
if rate_limit is not None and rolling_count > rate_limit:
Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
11
from __future__ import annotations
22

3-
import random
3+
import uuid
44

55
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import (
66
ValkeyRateLimitClient,
77
)
8+
from ai.backend.common.identifier.user import UserID
89

910

1011
async def test_valkey_rate_limit_logic_execution(
1112
test_valkey_rate_limit: ValkeyRateLimitClient,
1213
) -> None:
1314
"""Test rate limiting logic execution."""
14-
access_key = f"test-logic-{random.randint(1000, 9999)}"
15+
user_id = UserID(uuid.uuid4())
1516

1617
# Execute the rate limiting logic
1718
result = await test_valkey_rate_limit.execute_rate_limit_logic(
18-
access_key=access_key,
19+
user_id=user_id,
1920
window=60,
2021
)
2122

2223
assert result == 1 # First request should return 1
2324

2425
# Execute again
2526
result2 = await test_valkey_rate_limit.execute_rate_limit_logic(
26-
access_key=access_key,
27+
user_id=user_id,
2728
window=60,
2829
)
2930

3031
assert result2 == 2 # Second request should return 2
32+
33+
assert await test_valkey_rate_limit.get_rolling_count(user_id) == 2

tests/unit/manager/api/test_ratelimit.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import uuid
34
from dataclasses import dataclass
45
from typing import Any
56
from unittest.mock import AsyncMock, MagicMock
@@ -8,9 +9,12 @@
89
from aiohttp import web
910

1011
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
12+
from ai.backend.common.identifier.user import UserID
1113
from ai.backend.manager.api.rest.ratelimit.handler import _rlim_window, make_rlim_middleware
1214
from ai.backend.manager.errors.api import RateLimitExceeded
1315

16+
_USER_ID = UserID(uuid.UUID("12345678-1234-5678-1234-567812345678"))
17+
1418

1519
@dataclass
1620
class RateLimitSuccessCase:
@@ -65,16 +69,16 @@ def mock_request_anonymous(self) -> web.Request:
6569
def mock_request_authorized(self) -> web.Request:
6670
"""Mock request for authorized user."""
6771
request = MagicMock(spec=web.Request)
68-
keypair_data = {
69-
"rate_limit": 30000,
70-
"access_key": "AKIAIOSFODNN7EXAMPLE",
71-
}
72+
keypair_data = {"rate_limit": 30000}
73+
user_data = {"uuid": _USER_ID}
7274

7375
def getitem(key: Any) -> Any:
7476
if key == "is_authorized":
7577
return True
7678
if key == "keypair":
7779
return keypair_data
80+
if key == "user":
81+
return user_data
7882
return None
7983

8084
request.__getitem__ = MagicMock(side_effect=getitem)
@@ -162,7 +166,7 @@ async def test_authorized_query_within_rate_limit(
162166

163167
# Valkey should be called for authorized requests
164168
mock_valkey_client.execute_rate_limit_logic.assert_called_once_with(
165-
access_key="AKIAIOSFODNN7EXAMPLE",
169+
user_id=_USER_ID,
166170
window=_rlim_window,
167171
)
168172

@@ -211,6 +215,6 @@ async def test_authorized_query_exceeds_rate_limit(
211215

212216
# Valkey should still be called
213217
mock_valkey_client.execute_rate_limit_logic.assert_called_once_with(
214-
access_key="AKIAIOSFODNN7EXAMPLE",
218+
user_id=_USER_ID,
215219
window=_rlim_window,
216220
)

0 commit comments

Comments
 (0)