Skip to content

Commit 9530633

Browse files
committed
fix(merge-queue): keep the signing gate off the GraphQL budget + rate-gate
The signing gate (added in 292a532) called fetchUnsignedCommitCount — a GraphQL call — every tick a signing-repo head reached the clean path, INCLUDING while a re-sign run was already in flight. On PostHog (a huge, chronically rate-limited account) that meant a signature GraphQL call every 10s per head, which helped exhaust the App's GraphQL primary point budget (~25-min backoff) and stall the queue; the calls also sit behind waitIfBlocked, so on a mid-tick backoff they'd sleep up to 60s each and stack into a 5-min tick wedge. Guard the gate the same way the rest of the queue defers: - isBlocked(account) → defer the head (don't sleep behind waitIfBlocked). - run already in flight → hold 'fixing' without re-polling signatures every tick (the main budget drain). - graphqlBudget.shouldDefer → skip the signature fetch until the window recovers. - 403 net's signature lookup guarded by isBlocked too. The account still recovers its budget GitHub-side (~25 min); this stops the queue from re-exhausting it via per-tick signature polling.
1 parent 52ba5dc commit 9530633

2 files changed

Lines changed: 44 additions & 6 deletions

File tree

packages/backend/src/__tests__/mergeQueueProcessor.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,20 @@ describe('mergeQueueProcessor', () => {
10161016
expect(state.resignAttempts).toBe(1); // not bumped — didn't pile on a second run
10171017
expect(await countTasks(db)).toBe(1); // still just the in-flight one
10181018
expect(mergeSpy).not.toHaveBeenCalled();
1019+
expect(mockUnsignedCount).not.toHaveBeenCalled(); // no per-tick signature poll while a run is in flight
1020+
});
1021+
1022+
it('defers the signing check (no signature fetch, no merge) when the GraphQL budget is in reserve', async () => {
1023+
mockRequiresSigning.mockResolvedValue(true);
1024+
mockUnsignedCount.mockResolvedValue(2);
1025+
vi.spyOn(graphqlBudget, 'shouldDefer').mockReturnValue(true); // budget in reserve
1026+
const prId = await insertPr(db, { summary: cleanSummary() });
1027+
1028+
await mergeQueueProcessor.runOnce();
1029+
1030+
expect(mockUnsignedCount).not.toHaveBeenCalled(); // didn't spend a scarce point on signatures
1031+
expect(mergeSpy).not.toHaveBeenCalled(); // and didn't attempt the doomed merge
1032+
expect((await getPr(db, prId)).state).toBe('open'); // deferred to a later tick
10191033
});
10201034

10211035
it('blocks with the signing reason once the re-sign budget is spent', async () => {

packages/backend/src/services/mergeQueueProcessor.ts

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -777,14 +777,19 @@ class MergeQueueProcessor {
777777
// unsigned commits, the refusal is (at least partly) unsigned commits —
778778
// re-sign via the fix task, and record the requirement so every future
779779
// PR on this branch is handled proactively (learn-from-403).
780+
// Skip the signature lookup when the account is in a rate-limit backoff
781+
// (it would sleep behind waitIfBlocked / burn a scarce point) — fall
782+
// through to the normal refusal handling; a later tick re-derives it.
780783
let unsignedNow = 0;
781784
try {
782-
unsignedNow = await fetchUnsignedCommitCount({
783-
workspaceId: row.workspaceId,
784-
owner: row.owner,
785-
repo: row.repo,
786-
number: row.number,
787-
});
785+
if (!githubRateGate.isBlocked(githubService.accountKeyFor(row.workspaceId))) {
786+
unsignedNow = await fetchUnsignedCommitCount({
787+
workspaceId: row.workspaceId,
788+
owner: row.owner,
789+
repo: row.repo,
790+
number: row.number,
791+
});
792+
}
788793
} catch {
789794
// ignore — fall through to the normal refusal handling below
790795
}
@@ -972,10 +977,29 @@ class MergeQueueProcessor {
972977
position: number,
973978
runActive: boolean
974979
): Promise<HeadVerdict | null> {
980+
const accountKey = githubService.accountKeyFor(row.workspaceId);
981+
// Hard rate-limit backoff: the signature GraphQL call AND the merge that
982+
// follows would each sleep up to MAX_GATE_WAIT_MS behind `waitIfBlocked`,
983+
// and stacking those inside one head is what wedged the tick. Defer the head
984+
// to a later tick (mirrors processGroup's between-heads rate-gate deferral).
985+
if (githubRateGate.isBlocked(accountKey)) return 'advance';
986+
975987
const base = baseBranchOf(row);
976988
if (!(await requiresSignedCommits(row.workspaceId, row.owner, row.repo, base))) {
977989
return null; // repo/branch doesn't require signed commits — merge as normal
978990
}
991+
// A re-sign run is already in flight — don't re-poll signatures every tick
992+
// (a needless GraphQL call per tick, on a budget-starved account). Hold the
993+
// badge at 'fixing' and let the run finish; the next no-run tick re-checks.
994+
if (runActive) {
995+
await this.ensureStatus(row, state, 'fixing', position);
996+
return 'advance';
997+
}
998+
// GraphQL budget in reserve — don't spend a scarce point on the signature
999+
// fetch (or the doomed merge it gates). Defer until the window recovers; this
1000+
// is what keeps the queue's signing checks from tipping the account over.
1001+
if (graphqlBudget.shouldDefer(accountKey)) return 'advance';
1002+
9791003
let unsigned: number;
9801004
try {
9811005
unsigned = await fetchUnsignedCommitCount({

0 commit comments

Comments
 (0)