Skip to content

Commit e2063b5

Browse files
committed
fix(redis): keep the subscriber's live command deadline at 5s
ioredis has one `commandTimeout` for the handshake commands and every live command on the socket, so tightening it to diagnose dead handshakes faster also cut the initial SUBSCRIBE from 5s to 2s — on the one path where a rejection fails the whole run. A ready-but-slow server delays SUBSCRIBE too. Restore the 5s tolerance that subscribe has always had and derive the readiness budget from it: one dead handshake recovers in ~10.5s, so an 11.5s budget still lets ioredis's reconnect rescue the case that failed. The local test mock now delegates to the shared, drift-guarded mirror of the budget arithmetic so the derived number under test is the one production derives.
1 parent 98cac46 commit e2063b5

2 files changed

Lines changed: 28 additions & 23 deletions

File tree

apps/sim/lib/execution/execution-signal.test.ts

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,24 @@ vi.mock('ioredis', () => ({
4040
},
4141
}))
4242

43-
vi.mock('@/lib/core/config/redis', () => ({
44-
getConfiguredRedisUrl: () => {
45-
if (mockRedisUrl.error) throw mockRedisUrl.error
46-
return mockRedisUrl.value
47-
},
48-
getRedisConnectionDefaults: () => ({}),
49-
// The budget arithmetic itself is covered in redis.test.ts; here only the
50-
// resulting number matters, and the readiness test reads it back by name.
51-
coldConnectionBudgetMs: (inputs: unknown) => {
52-
budgetInputs.value = inputs
53-
return 10_000
54-
},
55-
}))
43+
vi.mock('@/lib/core/config/redis', async () => {
44+
const { redisConfigMock } = await import('@sim/testing')
45+
return {
46+
getConfiguredRedisUrl: () => {
47+
if (mockRedisUrl.error) throw mockRedisUrl.error
48+
return mockRedisUrl.value
49+
},
50+
getRedisConnectionDefaults: () => ({}),
51+
// Records the inputs, then answers with the shared mirror of the real
52+
// arithmetic so the derived budget here is the number production derives.
53+
coldConnectionBudgetMs: (
54+
inputs: Parameters<typeof redisConfigMock.coldConnectionBudgetMs>[0]
55+
) => {
56+
budgetInputs.value = inputs
57+
return redisConfigMock.coldConnectionBudgetMs(inputs)
58+
},
59+
}
60+
})
5661

5762
import {
5863
getExecutionSignalHub,
@@ -385,11 +390,11 @@ describe('ExecutionSignalHub', () => {
385390
retryStrategy: (attempt: number) => number
386391
}
387392

388-
// Two dead handshakes, so the budget states the reconnect delays after
389-
// attempt 1 and attempt 2 as the client's own retryStrategy would return them.
393+
// One dead handshake, so the budget states the reconnect delay after
394+
// attempt 1 as the client's own retryStrategy would return it.
390395
expect(budgetInputs.value).toEqual({
391396
commandTimeoutMs: options.commandTimeout,
392-
retryDelaysMs: [options.retryStrategy(1), options.retryStrategy(2)],
397+
retryDelaysMs: [options.retryStrategy(1)],
393398
})
394399
// And the wait must outlast at least one full dead attempt, or the retry is decorative.
395400
expect(SUBSCRIBER_READY_TIMEOUT_MS).toBeGreaterThan(

apps/sim/lib/execution/execution-signal.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@ import {
1111
const logger = createLogger('ExecutionSignalHub')
1212
const EXECUTION_SIGNAL_PREFIX = 'execution:signal:'
1313
/**
14-
* Tight, because this client only ever issues `SUBSCRIBE`/`UNSUBSCRIBE`, and
15-
* only once the connection is ready — sub-millisecond commands that never sit
16-
* in the offline queue behind a handshake. Its main job is bounding how long a
17-
* dead handshake takes to be diagnosed and torn down.
14+
* Bounds the live `SUBSCRIBE` as well as the handshake commands — ioredis has
15+
* one deadline for both — and an initial subscribe that rejects fails the run,
16+
* so this stays at the tolerance a ready-but-slow server has always been
17+
* given rather than being tightened to diagnose dead handshakes faster.
1818
*/
19-
const SUBSCRIBER_COMMAND_TIMEOUT_MS = 2_000
19+
const SUBSCRIBER_COMMAND_TIMEOUT_MS = 5_000
2020
const subscriberRetryDelayMs = (attempt: number): number => Math.min(attempt * 500, 5000)
2121
/**
22-
* Room for two dead handshakes and then a healthy one, so ioredis's own
22+
* Room for one dead handshake and then a healthy one, so ioredis's own
2323
* reconnect can be what rescues a stalled connection instead of the wait
2424
* expiring while the first attempt is still being diagnosed.
2525
*/
2626
export const SUBSCRIBER_READY_TIMEOUT_MS = coldConnectionBudgetMs({
2727
commandTimeoutMs: SUBSCRIBER_COMMAND_TIMEOUT_MS,
28-
retryDelaysMs: [1, 2].map(subscriberRetryDelayMs),
28+
retryDelaysMs: [subscriberRetryDelayMs(1)],
2929
})
3030
export const LEGACY_EXECUTION_CANCEL_CHANNEL = 'execution:cancel'
3131

0 commit comments

Comments
 (0)