Use listing-scoped concurrency group for PrCard creation on shared submission realm#4251
Conversation
58acd0e to
bb2ecb8
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a job-queue bottleneck in the submission flow by preventing all CreatePrCard (run-command) jobs targeting the shared /submissions/ realm from serializing under a single concurrency group. It introduces a caller-controlled concurrency group override so CreatePrCard jobs can be scoped by listing branch name, enabling parallel processing across different listings while still serializing same-listing operations.
Changes:
- Add an optional
concurrencyGroupoverride parameter toenqueueRunCommandJob. - Use a listing-scoped concurrency group for the
create-pr-cardjob in the bot-runner submission flow. - Extend bot-runner tests to assert the intended concurrency-group behavior across the three-job sequence.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/runtime-common/jobs/run-command.ts | Adds optional concurrency-group override for published run-command jobs. |
| packages/bot-runner/lib/command-runner.ts | Applies listing-scoped concurrency grouping for PR card creation in the shared submissions realm. |
| packages/bot-runner/tests/command-runner-test.ts | Adds assertions verifying default vs listing-scoped concurrency groups across the submission flow jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.queuePublisher, | ||
| this.dbAdapter, | ||
| userInitiatedPriority, | ||
| concurrencyGroup ? { concurrencyGroup } : undefined, |
There was a problem hiding this comment.
enqueueRunCommand drops the override when concurrencyGroup is an empty string because it uses a truthiness check (concurrencyGroup ? ...). Since the parameter type is string | undefined, an empty string is a valid value and should be forwarded (or rejected via validation). Consider switching to an explicit concurrencyGroup !== undefined check (and, if supported, forwarding null as well) so the function doesn’t silently fall back to the default group.
| concurrencyGroup ? { concurrencyGroup } : undefined, | |
| concurrencyGroup !== undefined ? { concurrencyGroup } : undefined, |
| export async function enqueueRunCommandJob( | ||
| args: RunCommandArgs, | ||
| queue: QueuePublisher, | ||
| _dbAdapter: DBAdapter, | ||
| priority: number, | ||
| opts?: { concurrencyGroup?: string }, | ||
| ) { | ||
| let job = await queue.publish<RunCommandResponse>({ | ||
| jobType: 'run-command', | ||
| concurrencyGroup: `command:${args.realmURL}`, | ||
| concurrencyGroup: opts?.concurrencyGroup ?? `command:${args.realmURL}`, | ||
| timeout: RUN_COMMAND_JOB_TIMEOUT_SEC, |
There was a problem hiding this comment.
enqueueRunCommandJob now supports a concurrency-group override, but the override type only allows string. The queue API supports concurrencyGroup: string | null, and allowing null here would let callers explicitly opt out of grouping (full parallelism) when needed. Consider widening opts.concurrencyGroup to string | null and passing it through as-is (treating undefined as “use default”).
bb2ecb8 to
a1cbb91
Compare
Summary
concurrencyGroupoverride toenqueueRunCommandJobfor callers that need finer-grained controlProblem
The job queue enforces that only one job per concurrency group runs at a time. All
run-commandjobs default tocommand:${realmURL}as their group. The submission flow has 3 sequential jobs:/submissions/realm (bottleneck)Job 2 is the problem: every user's PrCard creation shares
command:${submissionRealmURL}, so they all serialize. With a 60s timeout per job and N concurrent submissions, the Nth user waits up to N×60s.Solution
Override the concurrency group only for Job 2 using the listing's branch name:
command:${submissionRealm}:listing:${branchName}
Jobs 1 and 3 keep the default
command:${realmURL}— they already target per-user realms so there's no cross-user contention.The override is applied in
createAndLinkPrCardbecause that's the earliest point where we havebranchName(returned by the GitHub PR creation step). At theenqueueRunCommandJoblevel, we only have genericRunCommandArgswith no domain-specific context — so the caller decides when and how to override.command:{userRealm}(default)command:{submissionRealm}:listing:{branch}command:{userRealm}(default)Test plan