mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
test(server): bound and qualify decision expiry lifecycle
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
472711eba2
commit
63b58cc26f
5 files changed
+148
-11
No files matched your search
@@ -0,0 +1,94 @@
|
||||
# Decision TTL expiry while idle
|
||||
|
||||
Open decisions already contain an `expiresAt` deadline. The expiry worker now
|
||||
uses that timestamp instead of scanning TTL candidates on each maintenance tick.
|
||||
This slice owns time-based expiry only. Target invalidation, interrupted effect
|
||||
recovery, and retention remain separate duties.
|
||||
|
||||
## Producers and transaction ownership
|
||||
|
||||
Creation and bundle creation notify before the decision insert in their existing
|
||||
transaction. Existing decision updates share `notifyDecisionChange`, which now
|
||||
signals expiry as well as continuation delivery. Cancellation, dismissal, and
|
||||
execution claims therefore remove obsolete expiry deadlines. All production
|
||||
inserts and updates are in the decision owner; acknowledgement only changes
|
||||
terminal delivery metadata and cannot change an open expiry. No supported API
|
||||
currently edits an existing decision's expiry timestamp. A future such writer
|
||||
must use this same notification boundary.
|
||||
|
||||
The helper adds no recovery wrappers to routes. Outer transactions and nested
|
||||
savepoints retain the shared commit/rollback and uncertain-outcome protocol.
|
||||
Creation without a committed row cannot publish a business deadline. Startup
|
||||
reconstructs saved deadlines, including already-overdue work. Independent SQL
|
||||
writers and another active server process need their own wake contract.
|
||||
|
||||
`decision-expiry.ts` owns due selection, the next deadline, and expiry settlement.
|
||||
`decisions.ts` delegates expiry-on-use and the existing target-driven expiration
|
||||
transition to that owner. Target selection remains outside the TTL worker.
|
||||
Startup registers TTL work before the combined decision sweep. The periodic
|
||||
combined sweep opts out of TTL selection and cannot expire a row merely because
|
||||
its TTL elapsed. Its public default still includes TTL and preserves the
|
||||
configured combined-sweep batch limit.
|
||||
|
||||
## Deadline, settlement, and recovery
|
||||
|
||||
The next deadline is the earliest open `expiresAt`. A partial index on
|
||||
`(expires_at, id)` covers open decisions. The query selects one indexed row;
|
||||
retained terminal history does not enlarge the deadline lookup. Expiry is due
|
||||
when `expiresAt <= now`, including equality. The old combined path skipped the
|
||||
exact equality despite selecting that row; this migration fixes that boundary.
|
||||
|
||||
Each worker pass attempts at most 100 candidates ordered by expiry and ID.
|
||||
Its in-memory cursor advances across failures and wraps after the last page.
|
||||
Remaining overdue work retains a retry through the saved earliest deadline.
|
||||
Failures retain the existing 30-second retry throttle. New earlier creation or
|
||||
resolution of the earliest decision causes deadline reconciliation. When only a
|
||||
future expiry remains, the worker does not query again until that deadline or a
|
||||
committed change. No open decisions means no recurring expiry work.
|
||||
|
||||
Settlement requires the row to remain open. TTL settlement also rechecks that
|
||||
the saved deadline is due. Metadata is merged into the current row. The existing
|
||||
expiry activity and origin continuation are preserved. A deadline-driven expiry
|
||||
records `ttl`; target-driven expiration retains its own reason. When both causes
|
||||
are possible, the first successful terminal transition determines the reason.
|
||||
Continuation failures belong to the continuation worker after the decision is
|
||||
expired. Neither owner replays decision effects.
|
||||
|
||||
Drain and standby defer dispatch while retaining the request. The worker checks
|
||||
the coordinator's AbortSignal between candidates. Shutdown finishes the admitted
|
||||
expiry and leaves untouched candidates for restart. It does not process the
|
||||
whole backlog before stopping. There is no new queue table or permanent DB
|
||||
listener, and no provider call is held inside a new transaction.
|
||||
|
||||
## Limits
|
||||
|
||||
The disabled-heartbeat branch keeps its existing behavior and does not register
|
||||
this worker. Expiry-on-use remains enforced independently of background work.
|
||||
The in-process deadline is not an external Cloud alarm. Existing conservative
|
||||
sleep blockers remain; this migration does not permit an instance to sleep with
|
||||
unsupported future obligations. Worker quiet is not proof of host or database
|
||||
suspension.
|
||||
|
||||
## Verification
|
||||
|
||||
The combined selection passes 103 tests: 14 new real-PostgreSQL expiry worker
|
||||
cases, 50 decision service/continuation cases, and 39 startup cases. It covers
|
||||
empty and terminal-history quiet, actual create and bundle producers, exact
|
||||
expiry, moving the next deadline earlier, cancellation, restart, outer rollback
|
||||
and commit, first creation during drain, failed retry through drain, failed wake
|
||||
delivery, separate aggregate/TTL ownership, 101 candidates with a failed first
|
||||
row, creation during delivery, and shutdown/restart with outstanding candidates.
|
||||
The parent archive race and bounded continuation shutdown regressions also pass.
|
||||
|
||||
The first selection had 97 passes and one fixture failure. The fault test cleared
|
||||
its error collector before an already-admitted failed query had finished. It now
|
||||
checks all collected causes after recovery settles. The next selection passed
|
||||
101 cases; after the parent review repairs and bounded expiry/shutdown coverage,
|
||||
all 103 pass. No production hold, retry, or timeout was weakened.
|
||||
|
||||
Migration 0383 applies and replays successfully. With 40,000 synthetic decisions
|
||||
and one open row, the next-deadline query uses the partial index, reads two shared
|
||||
buffers, and executes in 0.022 ms locally. The probe transaction rolls back. This
|
||||
is query evidence, not a production latency or savings claim. Workspace
|
||||
typecheck and build pass on the combined branch. Full PR CI and fresh review
|
||||
remain required before readiness.
|
||||
@@ -410,3 +410,20 @@ admitted failure finished; the assertion now verifies all errors after settled
|
||||
recovery. No hold or retry was weakened. This branch must receive the parent
|
||||
review repairs and adapt its own paged expiry worker to cancellation before PR
|
||||
publication. No tenth PR or readiness is claimed.
|
||||
|
||||
|
||||
## Slice 10: decision TTL expiry
|
||||
|
||||
The TTL implementation now includes parent continuation repairs from `5f96892a58`.
|
||||
The merge preserves both the expiry owner and per-decision effect ownership.
|
||||
See [decision TTL expiry](../idle-decision-expiry.md). Creation and bundle writes
|
||||
join the same notification helper; an indexed minimum open expiry drives work.
|
||||
The combined tick opts out of TTL selection without removing target invalidation
|
||||
or effect recovery. Expiry-on-use remains enforced, including exact equality.
|
||||
Bounded passes retain a fair expiry/ID cursor and stop between candidates.
|
||||
|
||||
All 103 selected tests pass after the merge, including fourteen TTL worker cases.
|
||||
Migration replay and a 40,000-row history query probe pass (two buffers).
|
||||
Final workspace typecheck/build and fresh PR CI/review remain required.
|
||||
The parent continuation PR is awaiting fresh review/CI on `5f96892a58`, with both
|
||||
initial findings repaired and resolved. The full inventory remains in scope.
|
||||
@@ -196,7 +196,11 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
if NEW.id = '10000000-0000-4000-8000-000000000000' and NEW.status = 'expired' then raise exception 'expiry unavailable'; end if;
|
||||
return NEW; end $$`);
|
||||
await db.execute(sql`create trigger decision_expiry_fail before update on decisions for each row execute function decision_expiry_fail()`);
|
||||
await expect(service().sweepTtlExpiry()).rejects.toMatchObject({ cause: { message: "expiry unavailable" } });
|
||||
const recovery = service();
|
||||
await expect(recovery.sweepTtlExpiry()).rejects.toMatchObject({ cause: { message: "expiry unavailable" } });
|
||||
expect(wake).toHaveBeenCalledTimes(99);
|
||||
expect((await service().get(ids.at(-1)!))?.status).toBe("open");
|
||||
expect(await recovery.sweepTtlExpiry()).toEqual({ expired: 1 });
|
||||
expect((await service().get(ids.at(-1)!))?.status).toBe("expired");
|
||||
expect((await service().get(ids[0]))?.status).toBe("open"); expect(wake).toHaveBeenCalledTimes(100);
|
||||
await db.execute(sql`drop trigger decision_expiry_fail on decisions`);
|
||||
@@ -214,4 +218,26 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
await expired(secondId); await quiet(); expect(wake).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
|
||||
it("finishes an admitted expiry but leaves the next candidate for restart during shutdown", async () => {
|
||||
const first = await create(), second = await create();
|
||||
await db.update(decisions).set({ expiresAt: new Date(Date.now() - 1000) });
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>(resolve => { release = resolve; });
|
||||
wake.mockImplementationOnce(async () => gate);
|
||||
const starting = start();
|
||||
let stopped = false;
|
||||
let stopping: Promise<void>;
|
||||
try {
|
||||
await vi.waitFor(() => expect(wake).toHaveBeenCalledTimes(1));
|
||||
stopping = coordinator!.stop().then(() => { stopped = true; });
|
||||
await vi.advanceTimersByTimeAsync(60_000); expect(stopped).toBe(false);
|
||||
} finally { release(); }
|
||||
await starting; await stopping!; coordinator = undefined;
|
||||
expect(wake).toHaveBeenCalledTimes(1); expect(stopped).toBe(true);
|
||||
const states = await Promise.all([service().get(first.id), service().get(second.id)]);
|
||||
expect(states.map(row => row!.status).sort()).toEqual(["expired", "open"]);
|
||||
await start(); await quiet(); expect(wake).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
});
|
||||
@@ -10,7 +10,7 @@ export function registerDecisionExpiryWork(
|
||||
) {
|
||||
return registerDeadlineWork(coordinator, DELIVERY_QUEUES.decisionExpiry, {
|
||||
retryMs: 30_000, canRun,
|
||||
run: () => service.sweepTtlExpiry(),
|
||||
run: signal => service.sweepTtlExpiry(new Date(), signal),
|
||||
next: service.nextExpiryAt,
|
||||
});
|
||||
}
|
||||
@@ -8,6 +8,7 @@ type ExpiryReason = "ttl" | "target_gone" | "target_completed";
|
||||
const open = sql`${decisions.status} = 'open'`;
|
||||
|
||||
export function decisionExpiryService(db: Db, deliverContinuation: (id: string) => Promise<void>) {
|
||||
let recoveryCursor: { expiresAt: Date; id: string } | undefined;
|
||||
async function expire(decision: Decision, reason: ExpiryReason, now: Date) {
|
||||
const [updated] = await writeDecisionChange(db, async tx => tx.update(decisions).set({
|
||||
status: "expired", updatedAt: now,
|
||||
@@ -29,18 +30,17 @@ export function decisionExpiryService(db: Db, deliverContinuation: (id: string)
|
||||
.orderBy(asc(decisions.expiresAt), asc(decisions.id)).limit(limit);
|
||||
}
|
||||
|
||||
async function sweep(now = new Date()) {
|
||||
let cursor: { expiresAt: Date; id: string } | undefined;
|
||||
async function sweep(now = new Date(), signal?: AbortSignal) {
|
||||
let expired = 0;
|
||||
if (signal?.aborted) return { expired };
|
||||
const errors: unknown[] = [];
|
||||
for (;;) {
|
||||
const rows = await due(now, 100, cursor);
|
||||
for (const row of rows) {
|
||||
try { expired += await expire(row, "ttl", now); } catch (error) { errors.push(error); }
|
||||
}
|
||||
if (rows.length < 100) break;
|
||||
cursor = rows.at(-1)!;
|
||||
const rows = await due(now, 100, recoveryCursor);
|
||||
for (const row of rows) {
|
||||
if (signal?.aborted) return { expired };
|
||||
try { expired += await expire(row, "ttl", now); } catch (error) { errors.push(error); }
|
||||
recoveryCursor = { expiresAt: row.expiresAt, id: row.id };
|
||||
}
|
||||
if (rows.length < 100) recoveryCursor = undefined;
|
||||
if (errors.length === 1) throw errors[0];
|
||||
if (errors.length) throw new AggregateError(errors, "Decision expiry failed");
|
||||
return { expired };
|
||||
|
||||
Reference in new issue
Block a user