Skip to content

perf: actuation lock is claimed after the refresh it should prevent — 1,180 discarded passes in 2h #10174

Description

@JSONbored

The actuation lock is claimed AFTER the work it exists to avoid

processors.ts (~4405-4440), the publish-and-maintain pass:

if (otherRefreshReasons || reviewsCacheStale) {
  await refreshPullRequestDetails(env, repoFullName, prNumber).catch(() => undefined);   // GitHub reads
}

const actuationLock = await claimPrActuationLock(env, repoFullName, pr.number);          // "does another pass own this?"
if (!actuationLock.acquired) {
  await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_lock_contended",});
  throw new PrActuationLockContendedError(repoFullName, pr.number, "public-surface-publish");
}

The pass asks "is another pass already doing this?" after paying for the refresh. On contention every bit of that work is discarded and the job retries.

Volume

github_app.pr_public_surface_lock_contended is the single most frequent audit event on the Orb — 1,180 occurrences between 09:00 and 10:53 today, roughly 10 per minute, ahead of every other event by 2x:

 github_app.pr_public_surface_lock_contended   1180
 github_app.review_deferred_ci_pending          590
 github_app.miner_detection_cache_hit           529
 github_app.type_label_decision                 274

Each of those is a pass that did the pre-lock work and threw it away.

What it actually costs

Being precise rather than dramatic: refreshPullRequestDetails is itself cached — it consults getPullRequestDetailSyncState and reuses stored pull_request_files rows when the last successful sync already covered the PR's current head SHA. So a contention does not always cost a GitHub call.

What it always costs is the DB sync-state reads plus shouldRefreshFilesForPreMergeChecks, and on a cache miss it costs a token fetch plus the PR files/reviews fetch — precisely the case that arises on a busy PR, which is also precisely when contention is most likely. The two peak together.

This is the same window in which the installation exhausted its REST quota (API rate limit exceeded for installation ID 143010787, 10:43), which stalled 66 queue jobs behind deferred_by: rate_limit.

Fix

Claim the lock first, then refresh. The lock's stated purpose (#9013) is to make "does another pass already own this PR" one question with one answer for the whole publish-then-maintain unit — asking it before doing the expensive part is strictly better and changes no semantics.

Holding the lock across the refresh lengthens the hold slightly, which is already handled: #9467 renews the lock while work runs precisely because this unit can span an AI review far longer than a refresh.

Worth checking in the same pass

The second contention site (~7727-7750) has the same shape and should be reviewed for the same ordering.

Found while investigating why the Orb hit its GitHub rate limit; see #10170 for the wider audit.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions