Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,30 @@ everything still outstanding is captured below.

---

## Split the database migration runtime

*Origin: CodeRabbit review on PR #1845 (`src/shared/db/migrations.ts`).*

`src/shared/db/migrations.ts` already exceeded 700 lines before PR #1845 and now
also owns request-sized batches and lease-scoped completion. Split it without
changing behavior. A useful starting boundary is:

- Move lease acquisition, ownership checks, and release into a focused lock
module.
- Move migration marker and schema marker statement builders into a recording
module.
- Move retry and pending-migration execution into a runner module.
- Leave `initDb` and database-state routing in `migrations.ts` as thin
orchestration.

Keep the existing request round-trip count, lazy migration loading, partial
progress behavior, and lock failure tests unchanged. This is intentionally
separate from PR #1845 because moving the whole migration subsystem while
changing its locking and batching would make the safety fix much harder to
review and roll back.

---

## Booking unification — phases 3 & 4

*Origin: `booking-unification.md`, `booking-unification-phase2.md`.*
Expand Down
35 changes: 18 additions & 17 deletions src/shared/db/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ import {
} from "#shared/cache-registry.ts";
import { beginTransaction, wrapExecute } from "#shared/db/libsql-call.ts";
import {
addQueryLogEntry,
countDatabaseRoundTrip,
enforceTransactionRoundTripGuard,
isQueryLogEnabled,
logCompletedSql,
trackQuery,
trackSql,
} from "#shared/db/query-log.ts";
import { getEnv } from "#shared/env.ts";
import { namedError } from "#shared/named-error.ts";
Expand Down Expand Up @@ -245,7 +242,7 @@ const retryOnDatabaseLock = <T>(run: () => Promise<T>): Promise<T> =>
type StatementRunner<T> = (sql: string, args?: InValue[]) => Promise<T>;

const executeTrackedStatement: StatementRunner<ResultSet> = (sql, args) =>
trackQuery(sql, () =>
trackSql(sql, () =>
retryOnDatabaseLock(() =>
args ? getDb().execute({ args, sql }) : getDb().execute(sql),
),
Expand Down Expand Up @@ -460,21 +457,14 @@ const runBatch = async (
mode: TransactionMode,
invalidate: boolean,
): Promise<ResultSet[]> => {
const start = performance.now();
const sqls = statements.map(({ sql }) => sql);
// Batch writes serialize against the single SQLite writer like any other write,
// so a contended batch waits and retries (then surfaces DatabaseBusyError)
// rather than throwing raw SQLITE_BUSY — matching execute() and withTransaction.
const results = await retryOnDatabaseLock(() =>
getDb().batch(statements, mode),
const results = await trackSql(sqls, () =>
retryOnDatabaseLock(() => getDb().batch(statements, mode)),
);
if (isQueryLogEnabled()) {
const elapsed = performance.now() - start;
// Every statement shares the one round-trip window [start, start+elapsed],
// so the footer's wall-clock union counts that time once (not N times).
for (const stmt of statements) addQueryLogEntry(stmt.sql, elapsed, start);
}
for (const stmt of statements) {
void logCompletedSql(stmt.sql);
if (invalidate) invalidateForSql(stmt.sql);
}
return results;
Expand Down Expand Up @@ -527,8 +517,10 @@ export const executeBatch = async (
};

/** The slice of an open write transaction handed to a {@link withTransaction}
* callback: run statements with `execute`; commit/rollback are managed for you. */
* callback: run statements singly or as one batch; commit/rollback are managed
* for you. */
export type TxScope = {
batch: (statements: InStatement[]) => Promise<ResultSet[]>;
execute: (stmt: InStatement) => Promise<ResultSet>;
};

Expand All @@ -550,6 +542,15 @@ const runWriteTransactionOnce = async <T>(
const writtenSql: string[] = [];
let statementCount = 0;
const scope: TxScope = {
batch: (statements) => {
const sqls = statements.map((statement) =>
typeof statement === "string" ? statement : statement.sql,
);
writtenSql.push(...sqls);
statementCount += 1;
enforceTransactionRoundTripGuard(statementCount, sqls.join("; "));
return trackSql(sqls, () => tx.batch(statements));
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
execute: (stmt) => {
const sql = typeof stmt === "string" ? stmt : stmt.sql;
writtenSql.push(sql);
Expand All @@ -560,7 +561,7 @@ const runWriteTransactionOnce = async <T>(
enforceTransactionRoundTripGuard(statementCount, sql);
// Track transactional statements too, so reads inside the callback still
// show in the debug footer and count toward the N+1 guard.
return trackQuery(sql, () => tx.execute(stmt));
return trackSql(sql, () => tx.execute(stmt));
},
};
try {
Expand Down
Loading