Skip to content

Commit 22f03db

Browse files
committed
fix(execution): give per-user Redis budget keys a fixed window
The per-user byte budget refreshed its TTL on every accepted write, so for any user who never went a full TTL without writing, the key never expired. The per-execution data it accounted for kept expiring underneath it, so the counter accrued bytes Redis had already dropped and drifted toward the ceiling. On reaching it, every subsequent write for that user was rejected until they stopped writing for a full TTL — and since a rejected write does not refresh the TTL, it recovered on its own and then refilled. User keys now get a fixed window: the TTL is set when the key is created and never extended. Applied to all five Lua scripts that touch the key so the writers cannot drift apart. Execution-scoped keys keep sliding — they are refreshed on the same schedule as the data they account for, so their counter and bytes stay in step.
1 parent 20d6f49 commit 22f03db

5 files changed

Lines changed: 137 additions & 9 deletions

File tree

apps/sim/lib/execution/event-buffer.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ function isResetScript(script: string): boolean {
6868
return script.includes('retained_bytes') && script.includes('replayStartEventId')
6969
}
7070

71+
function countOccurrences(haystack: string, needle: string): number {
72+
return haystack.split(needle).length - 1
73+
}
74+
7175
describe('execution event buffer', () => {
7276
beforeEach(() => {
7377
vi.clearAllMocks()
@@ -417,6 +421,37 @@ describe('execution event buffer', () => {
417421
)
418422
})
419423

424+
it('never extends an existing user budget window while flushing events', async () => {
425+
let flushScript = ''
426+
mockRedis.eval.mockImplementation(async (script: string) => {
427+
if (isFlushScript(script)) flushScript = script
428+
return [1, false, 0]
429+
})
430+
431+
const writer = createExecutionEventWriter('exec-1', { userId: 'user-1' })
432+
await writer.writeTerminal(makeEvent('terminal'), 'complete')
433+
434+
expect(flushScript).not.toBe('')
435+
const userKeyExpires = countOccurrences(flushScript, "redis.call('EXPIRE', KEYS[5]")
436+
const userKeyTtlGuards = countOccurrences(flushScript, "redis.call('TTL', KEYS[5]) < 0")
437+
expect(userKeyExpires).toBeGreaterThan(0)
438+
expect(userKeyTtlGuards).toBe(userKeyExpires)
439+
})
440+
441+
it('keeps sliding the execution budget window, which expires with its own data', async () => {
442+
let flushScript = ''
443+
mockRedis.eval.mockImplementation(async (script: string) => {
444+
if (isFlushScript(script)) flushScript = script
445+
return [1, false, 0]
446+
})
447+
448+
const writer = createExecutionEventWriter('exec-1', { userId: 'user-1' })
449+
await writer.writeTerminal(makeEvent('terminal'), 'complete')
450+
451+
expect(countOccurrences(flushScript, "redis.call('TTL', KEYS[4]) < 0")).toBe(0)
452+
expect(countOccurrences(flushScript, "redis.call('EXPIRE', KEYS[4]")).toBeGreaterThan(0)
453+
})
454+
420455
it('reports pruned replay buffers before reading incomplete events', async () => {
421456
mockRedis.hgetall.mockResolvedValue({ status: 'active', earliestEventId: '10' })
422457

apps/sim/lib/execution/event-buffer.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ if net_bytes > 0 then
8585
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
8686
if #KEYS >= 5 then
8787
redis.call('INCRBY', KEYS[5], net_bytes)
88-
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
88+
if redis.call('TTL', KEYS[5]) < 0 then
89+
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
90+
end
8991
end
9092
elseif net_bytes < 0 then
9193
local release_bytes = -net_bytes
@@ -99,17 +101,14 @@ elseif net_bytes < 0 then
99101
local user_next = redis.call('DECRBY', KEYS[5], release_bytes)
100102
if user_next <= 0 then
101103
redis.call('DEL', KEYS[5])
102-
else
104+
elseif redis.call('TTL', KEYS[5]) < 0 then
103105
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
104106
end
105107
end
106108
else
107109
if redis.call('EXISTS', KEYS[4]) == 1 then
108110
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
109111
end
110-
if #KEYS >= 5 and redis.call('EXISTS', KEYS[5]) == 1 then
111-
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
112-
end
113112
end
114113
for i = 9, #ARGV, 2 do
115114
redis.call('ZADD', KEYS[1], ARGV[i], ARGV[i + 1])
@@ -148,7 +147,7 @@ if retained_bytes > 0 then
148147
local user_next = redis.call('DECRBY', KEYS[4], retained_bytes)
149148
if user_next <= 0 then
150149
redis.call('DEL', KEYS[4])
151-
else
150+
elseif redis.call('TTL', KEYS[4]) < 0 then
152151
redis.call('EXPIRE', KEYS[4], tonumber(ARGV[4]))
153152
end
154153
end
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
import {
6+
getExecutionRedisBudgetKeys,
7+
reserveExecutionRedisBytes,
8+
} from '@/lib/execution/redis-budget.server'
9+
10+
function countOccurrences(haystack: string, needle: string): number {
11+
return haystack.split(needle).length - 1
12+
}
13+
14+
async function captureReserveScript(userId?: string): Promise<string> {
15+
let script = ''
16+
const redis = {
17+
eval: vi.fn(async (source: string) => {
18+
script = source
19+
return [1, 'ok', 0, 0]
20+
}),
21+
}
22+
23+
await reserveExecutionRedisBytes(redis as never, {
24+
executionId: 'exec-1',
25+
userId,
26+
category: 'event_buffer',
27+
operation: 'write_events',
28+
bytes: 128,
29+
})
30+
31+
return script
32+
}
33+
34+
describe('reserveExecutionRedisBytes', () => {
35+
it('scopes the reservation to the execution, and to the user when one is known', () => {
36+
expect(
37+
getExecutionRedisBudgetKeys({
38+
executionId: 'exec-1',
39+
category: 'event_buffer',
40+
operation: 'write_events',
41+
bytes: 1,
42+
})
43+
).toEqual(['execution:redis-budget:execution:exec-1'])
44+
45+
expect(
46+
getExecutionRedisBudgetKeys({
47+
executionId: 'exec-1',
48+
userId: 'user-1',
49+
category: 'event_buffer',
50+
operation: 'write_events',
51+
bytes: 1,
52+
})
53+
).toEqual(['execution:redis-budget:execution:exec-1', 'execution:redis-budget:user:user-1'])
54+
})
55+
56+
/**
57+
* A user key aggregates across every execution that user runs, so extending
58+
* its TTL on each write keeps it alive indefinitely while the per-execution
59+
* data it accounts for expires underneath it. The window must be fixed.
60+
*/
61+
it('never extends an existing user budget window', async () => {
62+
const script = await captureReserveScript('user-1')
63+
64+
const userKeyExpires = countOccurrences(script, "redis.call('EXPIRE', KEYS[2]")
65+
const userKeyTtlGuards = countOccurrences(script, "redis.call('TTL', KEYS[2]) < 0")
66+
expect(userKeyExpires).toBeGreaterThan(0)
67+
expect(userKeyTtlGuards).toBe(userKeyExpires)
68+
})
69+
70+
it('keeps sliding the execution budget window, which expires with its own data', async () => {
71+
const script = await captureReserveScript('user-1')
72+
73+
expect(countOccurrences(script, "redis.call('TTL', KEYS[1]) < 0")).toBe(0)
74+
expect(countOccurrences(script, "redis.call('EXPIRE', KEYS[1]")).toBeGreaterThan(0)
75+
})
76+
})

apps/sim/lib/execution/redis-budget.server.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ const MAX_EXECUTION_REDIS_BYTES = 64 * 1024 * 1024
1212
const MAX_USER_REDIS_BYTES = 256 * 1024 * 1024
1313
const REDIS_BUDGET_TTL_SECONDS = 60 * 60
1414

15+
/**
16+
* Execution and user budget keys expire differently on purpose.
17+
*
18+
* An execution key accounts for data that is refreshed on the same schedule as
19+
* the key itself, so sliding its TTL on every write keeps the counter and the
20+
* bytes it represents in step.
21+
*
22+
* A user key aggregates across every execution that user runs. Sliding its TTL
23+
* on each write keeps it alive indefinitely for any user who stays active,
24+
* while the per-execution data it accounts for keeps expiring underneath it —
25+
* so the counter accrues bytes Redis has already dropped and eventually pins
26+
* the user at their ceiling until they go a full TTL without writing. User
27+
* keys therefore get a fixed window: the TTL is set when the key is created
28+
* and never extended.
29+
*/
30+
1531
const RESERVE_REDIS_BYTES_SCRIPT = `
1632
local bytes = tonumber(ARGV[1])
1733
local execution_limit = tonumber(ARGV[2])
@@ -32,7 +48,9 @@ redis.call('INCRBY', KEYS[1], bytes)
3248
redis.call('EXPIRE', KEYS[1], ttl_seconds)
3349
if #KEYS >= 2 then
3450
redis.call('INCRBY', KEYS[2], bytes)
35-
redis.call('EXPIRE', KEYS[2], ttl_seconds)
51+
if redis.call('TTL', KEYS[2]) < 0 then
52+
redis.call('EXPIRE', KEYS[2], ttl_seconds)
53+
end
3654
end
3755
return {1, 'ok', execution_current + bytes, user_current + bytes}
3856
`

apps/sim/lib/uploads/utils/user-file-base64.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ if bytes and bytes > 0 then
5959
local user_next = redis.call('DECRBY', KEYS[4], bytes)
6060
if user_next <= 0 then
6161
redis.call('DEL', KEYS[4])
62-
else
62+
elseif redis.call('TTL', KEYS[4]) < 0 then
6363
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
6464
end
6565
end
@@ -129,7 +129,7 @@ if #KEYS >= 4 then
129129
redis.call('DEL', KEYS[4])
130130
end
131131
end
132-
if redis.call('EXISTS', KEYS[4]) == 1 then
132+
if redis.call('EXISTS', KEYS[4]) == 1 and redis.call('TTL', KEYS[4]) < 0 then
133133
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
134134
end
135135
end

0 commit comments

Comments
 (0)