Skip to content

Commit 19707cc

Browse files
849261680velvet-shark
849261680
and
velvet-shark
authored
fix(cron): avoid gateway restart on setup timeout (#96396)
* fix(cron): avoid gateway restart on setup timeout * fix(cron): avoid gateway restart on setup timeout --------- Co-authored-by: Radek Sienkiewicz <[email protected]>
1 parent a3b4e81 commit 19707cc

8 files changed

Lines changed: 22 additions & 38 deletions

File tree

‎src/cron/active-jobs-manual-run.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ describe("cron activeJobIds — manual-run mark/clear", () => {
182182
}
183183
});
184184

185-
it("requests one setup-timeout restart when concurrent manual runs both stall before runner start", async () => {
185+
it("sends one setup-timeout notification when concurrent manual runs both stall before runner start", async () => {
186186
vi.useFakeTimers();
187187
const now = Date.parse("2025-12-13T17:00:00.000Z");
188188
vi.setSystemTime(now);

‎src/cron/service.test-harness.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ export function createMockCronStateForJobs(params: {
236236
stopped: false,
237237
restartRecoveryPending: false,
238238
activeManualRunJobIds: new Set<string>(),
239-
manualSetupTimeoutRestartNotified: false,
239+
manualSetupTimeoutNotified: false,
240240
timer: null,
241241
storeLoadedAtMs: nowMs,
242242
op: Promise.resolve(),

‎src/cron/service/ops.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ function clearManualCronJobActive(
8686
state.activeManualRunJobIds.delete(jobId);
8787
clearCronJobActive(jobId, activeJobMarker);
8888
if (state.activeManualRunJobIds.size === 0) {
89-
state.manualSetupTimeoutRestartNotified = false;
89+
state.manualSetupTimeoutNotified = false;
9090
}
9191
}
9292

@@ -98,11 +98,11 @@ function maybeNotifyManualIsolatedSetupTimeout(
9898
isolatedAgentSetupTimeout?: IsolatedAgentSetupTimeoutSignal;
9999
},
100100
): boolean {
101-
if (!result.isolatedAgentSetupTimeout || state.manualSetupTimeoutRestartNotified) {
101+
if (!result.isolatedAgentSetupTimeout || state.manualSetupTimeoutNotified) {
102102
return false;
103103
}
104104
const notified = maybeNotifyIsolatedAgentSetupTimeout(state, result);
105-
state.manualSetupTimeoutRestartNotified ||= notified;
105+
state.manualSetupTimeoutNotified ||= notified;
106106
return notified;
107107
}
108108

‎src/cron/service/state.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ export type CronServiceState = {
197197
stopped: boolean;
198198
restartRecoveryPending: boolean;
199199
activeManualRunJobIds: Set<string>;
200-
manualSetupTimeoutRestartNotified: boolean;
200+
manualSetupTimeoutNotified: boolean;
201201
/** Serializes mutating service operations so store writes and timers stay ordered. */
202202
op: Promise<unknown>;
203203
warnedDisabled: boolean;
@@ -221,7 +221,7 @@ export function createCronServiceState(deps: CronServiceDeps): CronServiceState
221221
stopped: false,
222222
restartRecoveryPending: false,
223223
activeManualRunJobIds: new Set<string>(),
224-
manualSetupTimeoutRestartNotified: false,
224+
manualSetupTimeoutNotified: false,
225225
op: Promise.resolve(),
226226
warnedDisabled: false,
227227
warnedInvalidPersistedJobKeys: new Set<string>(),

‎src/cron/service/timer.regression.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,7 +1301,7 @@ describe("cron service timer regressions", () => {
13011301
}
13021302
});
13031303

1304-
it("notifies setup-timeout restart after startup catch-up finalization", async () => {
1304+
it("notifies setup timeout after startup catch-up finalization", async () => {
13051305
vi.useFakeTimers();
13061306
try {
13071307
const store = timerRegressionFixtures.makeStorePath();
@@ -1926,7 +1926,7 @@ describe("cron service timer regressions", () => {
19261926
expect(jobs.find((job) => job.id === second.id)?.state.lastStatus).toBe("ok");
19271927
});
19281928

1929-
it("requests one setup-timeout restart when a concurrent cron batch stalls before runners start", async () => {
1929+
it("sends one setup-timeout notification when a concurrent cron batch stalls before runners start", async () => {
19301930
vi.useFakeTimers();
19311931
try {
19321932
const store = timerRegressionFixtures.makeStorePath();
@@ -1990,7 +1990,7 @@ describe("cron service timer regressions", () => {
19901990
}
19911991
});
19921992

1993-
it("requests setup-timeout restart after a prior serial cron job completes", async () => {
1993+
it("sends setup-timeout notification after a prior serial cron job completes", async () => {
19941994
vi.useFakeTimers();
19951995
try {
19961996
const store = timerRegressionFixtures.makeStorePath();
@@ -2058,7 +2058,7 @@ describe("cron service timer regressions", () => {
20582058
}
20592059
});
20602060

2061-
it("requests setup-timeout restart when manual and scheduled runs both stall", async () => {
2061+
it("sends setup-timeout notification when manual and scheduled runs both stall", async () => {
20622062
vi.useFakeTimers();
20632063
try {
20642064
const store = timerRegressionFixtures.makeStorePath();
@@ -2128,7 +2128,7 @@ describe("cron service timer regressions", () => {
21282128
}
21292129
});
21302130

2131-
it("suppresses scheduled rearm after manual setup-timeout restart request", async () => {
2131+
it("rearms scheduled jobs after manual setup timeout notification", async () => {
21322132
vi.useFakeTimers();
21332133
try {
21342134
const store = timerRegressionFixtures.makeStorePath();
@@ -2179,8 +2179,8 @@ describe("cron service timer regressions", () => {
21792179
await vi.advanceTimersByTimeAsync(1);
21802180

21812181
expect(onIsolatedAgentSetupTimeout).toHaveBeenCalledTimes(1);
2182-
expect(state.restartRecoveryPending).toBe(true);
2183-
expect(state.timer).toBeNull();
2182+
expect(state.restartRecoveryPending).toBe(false);
2183+
expect(state.timer).not.toBeNull();
21842184
expect(scheduledStarted).not.toHaveBeenCalled();
21852185
} finally {
21862186
vi.useRealTimers();
@@ -2352,7 +2352,7 @@ describe("cron service timer regressions", () => {
23522352
).toBe(replacementReservationMs);
23532353
});
23542354

2355-
it("stops an active scheduled batch from claiming more jobs after manual setup-timeout recovery", async () => {
2355+
it("continues an active scheduled batch after manual setup-timeout notification", async () => {
23562356
vi.useFakeTimers();
23572357
try {
23582358
const store = timerRegressionFixtures.makeStorePath();
@@ -2419,14 +2419,14 @@ describe("cron service timer regressions", () => {
24192419
await vi.advanceTimersByTimeAsync(60_100);
24202420
now += 60_100;
24212421
await manualRun;
2422-
expect(state.restartRecoveryPending).toBe(true);
2422+
expect(state.restartRecoveryPending).toBe(false);
24232423

24242424
finishFirstScheduled.resolve();
24252425
await timerRun;
24262426

24272427
const second = requireJob(state, secondScheduledJob.id);
24282428
expect(onIsolatedAgentSetupTimeout).toHaveBeenCalledTimes(1);
2429-
expect(secondScheduledStarted).not.toHaveBeenCalled();
2429+
expect(secondScheduledStarted).toHaveBeenCalledWith(secondScheduledJob.id);
24302430
expect(second.state.runningAtMs).toBeUndefined();
24312431
} finally {
24322432
vi.useRealTimers();
@@ -2794,7 +2794,7 @@ describe("cron service timer regressions", () => {
27942794
}
27952795
});
27962796

2797-
it("does not request setup-timeout restart for cron-nested lane contention", async () => {
2797+
it("does not notify setup timeout for cron-nested lane contention", async () => {
27982798
vi.useFakeTimers();
27992799
try {
28002800
const store = timerRegressionFixtures.makeStorePath();
@@ -2854,7 +2854,7 @@ describe("cron service timer regressions", () => {
28542854
}
28552855
});
28562856

2857-
it("does not notify setup-timeout restart for custom-session cron waits", async () => {
2857+
it("does not notify setup timeout for custom-session cron waits", async () => {
28582858
vi.useFakeTimers();
28592859
try {
28602860
const store = timerRegressionFixtures.makeStorePath();

‎src/cron/service/timer.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,6 @@ export function maybeNotifyIsolatedAgentSetupTimeout(
344344
if (!notified) {
345345
return false;
346346
}
347-
state.restartRecoveryPending = true;
348347
return true;
349348
}
350349

‎src/gateway/server-cron.test.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ describe("buildGatewayCronService", () => {
285285
});
286286
});
287287

288-
it("requests a safe gateway restart when isolated cron setup times out", async () => {
288+
it("backs off isolated cron setup timeout without gateway restart", async () => {
289289
vi.useFakeTimers();
290290
const cfg = createCronConfig("server-cron-isolated-setup-timeout");
291291
loadConfigMock.mockReturnValue(cfg);
@@ -315,12 +315,7 @@ describe("buildGatewayCronService", () => {
315315
const runResult = await runPromise;
316316

317317
expect(runResult).toEqual({ ok: true, ran: true });
318-
expect(requestSafeGatewayRestartMock).toHaveBeenCalledTimes(1);
319-
expect(requestSafeGatewayRestartMock).toHaveBeenCalledWith({
320-
reason: "cron.isolated_agent_setup_timeout",
321-
delayMs: 0,
322-
preservePendingEmitHooks: true,
323-
});
318+
expect(requestSafeGatewayRestartMock).not.toHaveBeenCalled();
324319
} finally {
325320
state.cron.stop();
326321
vi.useRealTimers();

‎src/gateway/server-cron.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import { formatErrorMessage } from "../infra/errors.js";
3232
import { resolveMainScopedEventSessionKey } from "../infra/event-session-routing.js";
3333
import { runHeartbeatOnce } from "../infra/heartbeat-runner.js";
3434
import { requestHeartbeat } from "../infra/heartbeat-wake.js";
35-
import { requestSafeGatewayRestart } from "../infra/restart-coordinator.js";
3635
import {
3736
consumeSelectedSystemEventEntries,
3837
enqueueSystemEventEntry,
@@ -547,23 +546,14 @@ export function buildGatewayCronService(params: {
547546
}).catch(() => {});
548547
},
549548
onIsolatedAgentSetupTimeout: ({ job, error, timeoutMs }) => {
550-
const restart = requestSafeGatewayRestart({
551-
reason: "cron.isolated_agent_setup_timeout",
552-
delayMs: 0,
553-
preservePendingEmitHooks: true,
554-
});
555549
cronLogger.warn(
556550
{
557551
jobId: job.id,
558552
jobName: job.name,
559553
timeoutMs,
560554
error,
561-
restartStatus: restart.status,
562-
restartCoalesced: restart.restart.coalesced,
563-
restartSummary: restart.preflight.summary,
564-
restartDelayMs: restart.restart.delayMs,
565555
},
566-
"cron: isolated agent setup timed out before runner start; requested safe gateway restart",
556+
"cron: isolated agent setup timed out before runner start; backing off job without gateway restart",
567557
);
568558
},
569559
sendCronFailureAlert: async ({ job, text, channel, to, mode, accountId }) =>

0 commit comments

Comments
 (0)