Commit d493e9c
authored
[CRCR] Raise default callback rate limit to 60/min/repo (#8203)
## Summary
The cross-repo CI relay **callback** Lambda (`crcr-callback-prod` /
`cross_repo_ci_callback`) enforces a **per-repo** sliding-window rate
limit in `utils/redis_helper.check_rate_limit`, defaulting to **20
requests/min** when `RATE_LIMIT_PER_MIN` is unset:
```python
# utils/config.py
rate_limit_per_min = int(os.getenv("RATE_LIMIT_PER_MIN", "20"))
```
No env override is deployed (the deploy Makefile only runs
`update-function-code`, and no IaC sets the variable), so the Lambda
runs at the in-code default of 20/min. That 429s legitimate job-fan-out
bursts — e.g. a downstream reporting `in_progress` for many jobs near
workflow start — which is what trips the L2 edge-case tests for
`TorchedHat/pytorch-redhat-ci`:
```
##[error]Callback server returned HTTP 429.
{"detail": "rate limit exceeded for TorchedHat/pytorch-redhat-ci"}
```
## Change
- `utils/config.py`: raise the default `RATE_LIMIT_PER_MIN` from `20` →
`60` (1 callback/sec/repo). An env var still overrides it for
per-deployment tuning.
- `utils/hud.py`: guard the user-facing "retry after N seconds" message
with `max(1, 60 // rate_limit_per_min)` so it stays sensible if the
limit is ever set above 60 (otherwise `60 // 61 == 0`).
## Why 60 is safe
- The limit is **per-repo** (key `oot:rate:{repo}`), so aggregate
downstream load = `60 × (#L2+ repos)`. There are currently 2 L2 repos
(`pytorch/crcr-test`, `TorchedHat/pytorch-redhat-ci`), so worst case ≈ 2
callbacks/sec — trivial for the HUD API / DynamoDB, even accounting for
the up-to-4× amplification from `HUD_MAX_RETRIES` during a HUD slowdown.
- Still a real abuse guard at 1 callback/sec/repo.
- The Redis ops per callback are negligible; the only meaningful
downstream cost is the single HUD POST, which this comfortably bounds.
## Deploy note
Only the **callback** Lambda evaluates the limit (`webhook` never calls
`check_rate_limit`), so only `cross_repo_ci_callback` needs redeploying:
```
make -C aws/lambda/cross_repo_ci_relay deploy-callback CALLBACK_FUNCTION_NAME=cross_repo_ci_callback
```
## Test plan
- Existing unit tests unaffected: `test_callback_handler.py` /
`test_redis_helper.py` set `cfg.rate_limit_per_min` explicitly per-test
(independent of the default); `test_config.py` does not assert the
default value.
- AI assistance (Claude) was used to prepare this change.
---------
Signed-off-by: Andrey Talman <atalman@fb.com>1 parent 50358ee commit d493e9c
3 files changed
Lines changed: 9 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
11 | 16 | | |
12 | 17 | | |
13 | 18 | | |
14 | 19 | | |
| 20 | + | |
15 | 21 | | |
16 | 22 | | |
17 | 23 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
140 | 140 | | |
141 | 141 | | |
142 | 142 | | |
143 | | - | |
| 143 | + | |
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
100 | | - | |
| 100 | + | |
101 | 101 | | |
102 | 102 | | |
103 | 103 | | |
| |||
0 commit comments