mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
perf(workspaces): reserve cohort members concurrently (#20401)
## Thinking Path > - Paperclip controls agent work and preserves its workspaces. > - The online importer moves up to eight shared-source leaves under one source fence. > - Each member must have a durable reservation before capture starts. > - Serial reservations repeat the same validation cost before any archive work. > - This pull request reserves members in parallel through their existing independent transactions. > - Every reservation finishes before an error can release the fence or start capture. ## Linked Issues or Issue Description Refs: #19873, #20387, #20394. **Current behavior** A measured six-member cohort spent about 110 seconds in serial reservations. Each reservation keeps its existing source, task, recovery and journal checks. **Proposed behavior** Start the bounded member reservations together. Join all outcomes, retain member order, and refuse capture if any member fails. Keep the exact partial-journal resume contract. **Reason and benefit** This removes serial metadata work before capture. Hosted speedup has not been measured for this change. ## What Changed - Replace only the cohort reservation caller loop with `Promise.allSettled`. - Preserve the existing per-member transaction, preassigned operation ID, source fingerprint and all source fences. - Extend the real PostgreSQL failure fixture to require overlapping reservations and complete peer closure before fence release. - Document the unchanged partial-journal and atomic publication contract. ## Verification - Canonical cached worktree preflight and plugin SDK prerequisites passed on Node 24.22.2. - The unchanged production baseline failed the new reservation overlap assertion: one reader entered instead of two. The canonical PostgreSQL wrapper cleaned up. The other 21 tests were intentionally not selected. - The first baseline command failed before PostgreSQL because root-level `tsx` resolution was absent. The corrected command uses the checkout's explicit `cli/node_modules/tsx/dist/loader.mjs`. - The four focused PostgreSQL controls passed before the final rebase: overlap/join failure, exact partial-journal resume, parallel capture with both cold restores, and atomic outer-commit rollback. Eighteen tests were intentionally not selected. Server TypeScript passed. - The final branch is based on merged #20394 at `302ada90dfda0e8fbcd8a514d506d310c9843bcb`. The two directly affected PostgreSQL reservation/resume controls passed again on final head `06d436e45705da3b616a2b91725960f6e0ea52c0` (2 passed, 20 intentionally not selected; 100.56 seconds). Server TypeScript passed again. The canonical wrapper completed cleanup. - The separate Ponytail review remains 4/5. Its nonblocking fan-out warning was resolved as the documented phase contract; no source change was required. Optional multi-error aggregation is outside this change. - Fresh Greptile review is 5/5 on `06d436e45705da3b616a2b91725960f6e0ea52c0`, with no actionable findings or unresolved threads: https://github.com/paperclipai/paperclip/pull/20401#issuecomment-6754025480. - Exact-head CI is complete: 56 success, 4 skipped and 1 neutral, with no failed or pending checks. Aggregate `ci / verify` passed: https://github.com/paperclipai/paperclip/actions/runs/42718439355. - Reproduce the final focused cases on Node 24: `node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts -t "joins concurrent reservation validation readers|resumes the exact membership after reservation stops halfway"`. Run `pnpm --filter @paperclipai/server exec tsc --noEmit` for TypeScript. ## Risks - API phase contract: `concurrency` limits capture workers. Preflight, reservation and post-finalizer validation fan out over the exact selected member set, with the canonical maximum of eight. The original DB pool bounds connections. `concurrency: 1` does not promise one metadata query or transaction at a time. - Up to eight reservations can use independent pool connections at once. Members that pass can leave durable journals when a peer fails. The existing exact-member resume path retains those journals; this change does not retry automatically. - Capture and publication remain blocked until every reservation settles. Publication stays serial in one transaction. Saved journal schemas, singleton behavior and recovery proof checks do not change. - This does not change concurrency inside the publication transaction. One PostgreSQL connection serializes that SQL, and changing its row-lock order has no measured benefit here. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model identifier or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
d95fdd8d79
commit
bf818280f2
3 files changed
+15
-7
No files matched your search
@@ -38,6 +38,10 @@ their existing reconstruction path.
|
||||
Preparation, lifecycle, runtime-service, expiration, and reopen owners retain
|
||||
the same physical keys before filesystem mutation or lifecycle exclusion. The
|
||||
importer holds all cohort task/project and lifecycle keys through publication.
|
||||
Each cohort reserves its at most eight members in independent transactions in
|
||||
parallel. It joins every reservation before starting capture or releasing the
|
||||
source fence, including on failure. Partial journals keep the full exact member
|
||||
and operation set for resume; publication stays serial in one transaction.
|
||||
It also holds the existing per-run physical finalizer keys for each exact native
|
||||
recovery proof. It re-inspects the complete source proof before capture and
|
||||
publication. Changed descriptors, allocations, owners, aliases, or Git metadata
|
||||
|
||||
@@ -313,10 +313,10 @@ describe.skipIf(!support.supported)("bounded workspace cohort migration", () =>
|
||||
expect.soft(await db.select().from(workspaceOperations)).toHaveLength(1);
|
||||
await assertLocal(f);
|
||||
});
|
||||
it.each(["preflight", "recovery"] as const)("joins concurrent %s validation readers before releasing the fence", async phase => {
|
||||
it.each(["preflight", "reservation", "recovery"] as const)("joins concurrent %s validation readers before releasing the fence", async phase => {
|
||||
const f = await fixture(), held = await recoverable(f), inspect = onlineWorkspaceSource.inspectOnlineWorkspaceSource;
|
||||
const counts = new Map<string, number>(), selected = new Set(f.leaves.map(row => row.id));
|
||||
const phaseCall = phase === "preflight" ? 2 : 4;
|
||||
const phaseCall = phase === "preflight" ? 2 : phase === "reservation" ? 3 : 4;
|
||||
let entered = 0, settled = false, bothEntered!: () => void, release!: () => void, firstFailed!: () => void;
|
||||
const both = new Promise<void>(resolve => { bothEntered = resolve; }), peer = new Promise<void>(resolve => { release = resolve; });
|
||||
const failed = new Promise<void>(resolve => { firstFailed = resolve; }), timer = setTimeout(() => bothEntered(), 5000);
|
||||
@@ -344,7 +344,8 @@ describe.skipIf(!support.supported)("bounded workspace cohort migration", () =>
|
||||
await expect(withWorkspaceSourceLocks(db, { companyId: f.companyId, projectIds: [f.projectId] }, async () => true, true)).resolves.toBe(true);
|
||||
expect((await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, held.runId)))[0]).toEqual(held.owner);
|
||||
await assertLocal(f);
|
||||
expect(await db.select().from(workspaceOperations)).toHaveLength(phase === "preflight" ? 0 : 2);
|
||||
expect(await db.select().from(workspaceOperations)).toHaveLength(phase === "preflight" ? 0 : phase === "reservation" ? 1 : 2);
|
||||
expect(await db.select().from(executionWorkspaceSnapshots)).toHaveLength(0);
|
||||
expect(await db.select().from(activityLog)).toHaveLength(0);
|
||||
});
|
||||
it.each(["failure", "cancel"] as const)("joins active capture peers and keeps the fence on %s", async mode => {
|
||||
|
||||
@@ -773,13 +773,16 @@ export async function migrateRemoteWorkspaceCohort(input: {
|
||||
if (!await tryAcquireExecutionWorkspaceLifecycleLock(lock, workspaceId)) throw new IsolatedRepositorySourceBusy("workspace_source_busy");
|
||||
}
|
||||
await preflight(input.db);
|
||||
const plans: MigrationPlan[] = [];
|
||||
for (const member of members) {
|
||||
const reservations = await Promise.allSettled(members.map(async member => {
|
||||
const reserved = await reserveMigration(member.input, member.onlineSource, cohort,
|
||||
fingerprint(initial.find(workspace => workspace.id === member.input.workspaceId)!));
|
||||
if (reserved.authority) throw new Error("workspace_cohort_membership_changed");
|
||||
plans.push({ ...member, reserved });
|
||||
}
|
||||
return { ...member, reserved };
|
||||
}));
|
||||
const plans: MigrationPlan[] = reservations.map(reservation => {
|
||||
if (reservation.status === "rejected") throw reservation.reason;
|
||||
return reservation.value;
|
||||
});
|
||||
await lockMigrationRecovery(lock, plans, cohort);
|
||||
const captures: Array<CapturedMigration | undefined> = Array(plans.length);
|
||||
const childVerifier = createMigrationChildVerifier(plans[0].input, plans[0].onlineSource, lock);
|
||||
|
||||
Reference in new issue
Block a user