diff --git a/doc/testing-native-remote-continuity.md b/doc/testing-native-remote-continuity.md index 639d1b27e3a..65671f91edf 100644 --- a/doc/testing-native-remote-continuity.md +++ b/doc/testing-native-remote-continuity.md @@ -184,14 +184,21 @@ local file becomes a marker that requires restoration from the external checkpoint before restart. Ordinary control changes do not rewrite a second local journal. The sandbox runner keeps its own journal. -Checkpoint transactions share a separate pool capped at two connections per +Checkpoint transactions share a separate pool capped at three connections per server database client. They queue without occupying the application pool or the existing lease-renewal pool. This can delay a control acknowledgement during a burst; it does not release that acknowledgement before its commit. The cost is -up to two additional active database connections per instance, not per agent. +up to three additional active database connections per instance, not per agent. There is no new SQL per text chunk or while idle, and the number of control transactions is unchanged. SQL cost collection must include the checkpoint pool. +The cap follows a bounded 2 → 3 → 4 → 2 diagnostic: both cap-2 trials failed +the existing burst-drain gate; 3 was the smallest tested passing cap. Retain +those failures and the diagnostic separately. The subsequent native feature-cost +comparison uses the common current DB factory and equal cap-3 policy for both +variants, so its baseline is a controlled baseline rather than the unmodified +historical cap-2 configuration. All existing benchmark gates remain unchanged. + A sparse checkpoint is marked `checkpointEventPolicy: "control-v1"`. Its peer must advertise `recoveryCursorVersion: 1`. During reconnection the controller echoes the runner's durable `resume.ackedSourceSeq` as `recoveryAckedSourceSeq` in the signed diff --git a/packages/db/src/checkpoint-pool.test.ts b/packages/db/src/checkpoint-pool.test.ts index 05dd02a8208..e75842053b6 100644 --- a/packages/db/src/checkpoint-pool.test.ts +++ b/packages/db/src/checkpoint-pool.test.ts @@ -1,5 +1,5 @@ import { afterEach, expect, it } from "vitest"; -import { checkpointDb, closeCheckpointDb, closeRegisteredClients, createDb, type Db } from "./client.js"; +import { checkpointDb, closeCheckpointDb, closeRegisteredClients, createDb, livenessDb, type Db } from "./client.js"; // postgres.js pools are lazy: these tests exercise actual client lifetimes but // never connect to a database or depend on embedded Postgres availability. @@ -17,6 +17,7 @@ it("fences checkpoints when shutdown precedes the first pool request", async () it("keeps an already-created checkpoint pool ended after shutdown", async () => { const db = createDb(url); const pool = checkpointDb(db); + expect(pool.$client.options.max).toBe(3); await closeCheckpointDb(db, 1); expect(checkpointDb(db)).toBe(pool); await expect(pool.$client`select 1`).rejects.toMatchObject({ code: "CONNECTION_ENDED" }); @@ -30,3 +31,16 @@ it("preserves other owner lifetimes and non-owning test-double behavior", async await closeCheckpointDb(nonOwner, 1); expect(checkpointDb(nonOwner)).toBe(nonOwner); }); + +it("bounds one cached checkpoint pool per owner without changing request or renewal capacity", () => { + const first = createDb(url, { maxConnections: 10 }); + const second = createDb(url, { maxConnections: 10 }); + const pool = checkpointDb(first); + expect(pool).not.toBe(first); + expect(checkpointDb(first)).toBe(pool); + expect(checkpointDb(second)).not.toBe(pool); + expect(pool.$client.options.max).toBe(3); + expect(checkpointDb(second).$client.options.max).toBe(3); + expect(first.$client.options.max).toBe(10); + expect(livenessDb(first).$client.options.max).toBe(2); +}); diff --git a/packages/db/src/client.ts b/packages/db/src/client.ts index b43d472d19e..66ff29b3aa6 100644 --- a/packages/db/src/client.ts +++ b/packages/db/src/client.ts @@ -295,9 +295,10 @@ export async function closeLivenessDb(db: Db, timeoutSeconds: number): Promise Db>(); const checkpointDbs = new WeakMap(); +const CHECKPOINT_POOL_MAX_CONNECTIONS = 3; const closedCheckpointOwners = new WeakSet(); /** The bounded controller checkpoint pool; transactions and test doubles keep @@ -342,7 +343,7 @@ function createDbClient(url: string, resolved: DatabaseClientOptions, owner?: ob ...resolved, maxConnections: LIVENESS_POOL_MAX_CONNECTIONS, applicationName: "paperclip-controller-lease", }, db)); checkpointDbFactories.set(db, () => createDbClient(url, { - ...resolved, maxConnections: 2, applicationName: "paperclip-runner-checkpoint", + ...resolved, maxConnections: CHECKPOINT_POOL_MAX_CONNECTIONS, applicationName: "paperclip-runner-checkpoint", }, db)); return db; } diff --git a/scripts/benchmark-native-controller-recovery.ts b/scripts/benchmark-native-controller-recovery.ts index ef8fd3efb49..8681954b34c 100644 --- a/scripts/benchmark-native-controller-recovery.ts +++ b/scripts/benchmark-native-controller-recovery.ts @@ -932,7 +932,7 @@ try { postgresMemoryLimitMiB: 1536, mainPool: 10, livenessPool: 2, - candidateCheckpointPool: 2, + candidateCheckpointPool: 3, }, workload: { mode, @@ -943,7 +943,8 @@ try { controlPayloadBytes: 4096, phaseSeconds: 65, snapshotTransactionsPerControl: 1, - baselineCheckpointPool: 2, + baselineCheckpointPool: 3, + baselinePoolPolicy: "Controlled baseline uses the common current DB factory's checkpoint cap of 3; this is not the unmodified historical cap-2 baseline.", baselineRenewalIntervalSeconds: 20, candidateRenewalIntervalSeconds: 20, historicalIssues: 10000,