perf(db): GIN trigram-индексы строятся CONCURRENTLY вне транзакции
2 GIN trigram-индекса (pages.title, pages.text_content) + users.name строились plain CREATE INDEX внутри Kysely-транзакции миграции: SHARE-lock блокирует записи на pages/users на минуты при автодеплое. Kysely гоняет миграцию в транзакции, а CONCURRENTLY внутри транзакции нельзя. Поэтому ensureConcurrentIndexes (concurrent-indexes.ts) пре-строит эти индексы через CREATE INDEX CONCURRENTLY (raw, вне транзакции) ДО миграатора — на существующей БД миграционный CREATE INDEX IF NOT EXISTS становится no-op и лок не берётся. Best-effort: на свежей БД (нет pages/f_unaccent) пре-build молча пропускается, а миграция строит индекс на пустой таблице. Худший случай = прежнее поведение, лучший — без лока. CONCURRENT_INDEXES — канонические определения; drift-guard тест сверяет их с выражениями в миграциях. Тест раннера: CONCURRENTLY+IF NOT EXISTS, вне транзакции, best-effort (падение одного не рвёт остальные). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,89 @@
|
||||
import * as path from 'path';
|
||||
import { readFileSync } from 'fs';
|
||||
|
||||
// Mock ONLY kysely's `sql.raw(...).execute()` so we can observe what
|
||||
// ensureConcurrentIndexes runs and how it handles failures, without a DB.
|
||||
const execMock = jest.fn((_db: unknown) => Promise.resolve(undefined));
|
||||
const rawMock = jest.fn((_stmt: string) => ({ execute: execMock }));
|
||||
jest.mock('kysely', () => {
|
||||
const actual = jest.requireActual('kysely');
|
||||
return { ...actual, sql: { ...actual.sql, raw: rawMock } };
|
||||
});
|
||||
|
||||
import { CONCURRENT_INDEXES, ensureConcurrentIndexes } from './concurrent-indexes';
|
||||
|
||||
describe('ensureConcurrentIndexes', () => {
|
||||
const fakeDb = { __topLevelKysely: true } as never;
|
||||
|
||||
beforeEach(() => {
|
||||
execMock.mockReset().mockResolvedValue(undefined);
|
||||
rawMock.mockClear();
|
||||
});
|
||||
|
||||
it('runs every registered index CONCURRENTLY, IF NOT EXISTS, outside a transaction', async () => {
|
||||
const onLog = jest.fn();
|
||||
await ensureConcurrentIndexes(fakeDb, onLog);
|
||||
|
||||
expect(rawMock).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length);
|
||||
for (const call of rawMock.mock.calls) {
|
||||
const stmt = call[0] as string;
|
||||
expect(stmt).toContain('CREATE INDEX CONCURRENTLY');
|
||||
expect(stmt).toContain('IF NOT EXISTS');
|
||||
}
|
||||
// Executed against the top-level db (a transaction would forbid CONCURRENTLY).
|
||||
for (const call of execMock.mock.calls) {
|
||||
expect(call[0]).toBe(fakeDb);
|
||||
}
|
||||
expect(onLog).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length);
|
||||
});
|
||||
|
||||
it('is best-effort: a failing index does not abort the rest and is reported', async () => {
|
||||
// Fail the FIRST index; the remaining ones must still be attempted.
|
||||
execMock
|
||||
.mockRejectedValueOnce(new Error('relation "pages" does not exist'))
|
||||
.mockResolvedValue(undefined);
|
||||
const onLog = jest.fn();
|
||||
|
||||
await expect(
|
||||
ensureConcurrentIndexes(fakeDb, onLog),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(execMock).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length);
|
||||
// The failure surfaced with an error argument for the caller to log.
|
||||
const errored = onLog.mock.calls.filter((c) => c[1] !== undefined);
|
||||
expect(errored).toHaveLength(1);
|
||||
expect(String(errored[0][1])).toContain('does not exist');
|
||||
});
|
||||
});
|
||||
|
||||
// DRIFT GUARD: each CONCURRENT_INDEXES entry pre-builds an index that a plain
|
||||
// migration ALSO creates with `IF NOT EXISTS`. If the two expressions diverge,
|
||||
// Postgres would treat them as different indexes and the pre-build would NOT
|
||||
// make the migration a no-op. Assert the migration files still contain each
|
||||
// index's functional expression.
|
||||
describe('CONCURRENT_INDEXES parity with the migrations', () => {
|
||||
const migrationsDir = path.join(__dirname, 'migrations');
|
||||
const files = [
|
||||
'20260705T120000-perf-indexes.ts',
|
||||
'20260706T120000-search-lookup-trgm.ts',
|
||||
].map((f) => readFileSync(path.join(migrationsDir, f), 'utf8'));
|
||||
const allMigrationSrc = files.join('\n');
|
||||
|
||||
it.each(CONCURRENT_INDEXES.map((i) => [i.name, i]))(
|
||||
'migration source still creates %s with the same expression',
|
||||
(_name, idx) => {
|
||||
// Extract the `USING gin ((...expr...) gin_trgm_ops)` tail from the
|
||||
// canonical create and assert the migration source contains it verbatim.
|
||||
const m = (idx as { create: string }).create.match(
|
||||
/ON \w+ (USING gin .+)$/,
|
||||
);
|
||||
expect(m).not.toBeNull();
|
||||
const expr = (m as RegExpMatchArray)[1];
|
||||
expect(allMigrationSrc).toContain(expr);
|
||||
// And the migration must build it by the SAME index name.
|
||||
expect(allMigrationSrc).toContain(
|
||||
`CREATE INDEX IF NOT EXISTS ${(idx as { name: string }).name}`,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,84 @@
|
||||
import { Kysely, sql } from 'kysely';
|
||||
|
||||
/**
|
||||
* Indexes that MUST be built with `CREATE INDEX CONCURRENTLY` so an auto-deploy
|
||||
* migration never takes a `SHARE` lock that blocks writes on a hot table
|
||||
* (`pages`) for the — potentially minutes-long — GIN trigram build (#495 item 12).
|
||||
*
|
||||
* Kysely runs each migration INSIDE a transaction (Postgres has transactional
|
||||
* DDL), and `CREATE INDEX CONCURRENTLY` cannot run inside a transaction block, so
|
||||
* these cannot live in an ordinary migration. Instead {@link ensureConcurrentIndexes}
|
||||
* builds them out-of-band (no transaction) BEFORE the migrator runs; the matching
|
||||
* migrations keep a plain `CREATE INDEX IF NOT EXISTS` as a backstop, which then
|
||||
* no-ops because the index already exists. So:
|
||||
* - existing prod DB, incremental deploy: pre-build runs CONCURRENTLY (no write
|
||||
* lock), migration's IF NOT EXISTS no-ops → the write-blocking build is gone;
|
||||
* - fresh DB (or a DB that has not yet created `pages` / `f_unaccent`): the
|
||||
* pre-build fails and is swallowed (best-effort), and the migration builds the
|
||||
* index normally on an empty/small table where the lock is irrelevant.
|
||||
* Worst case therefore equals the previous behaviour; best case removes the lock.
|
||||
*
|
||||
* The `create` text is the CANONICAL definition — it MUST match the migration's
|
||||
* `IF NOT EXISTS` create expression exactly (same functional expression + opclass)
|
||||
* or Postgres would treat them as two different indexes.
|
||||
*/
|
||||
export const CONCURRENT_INDEXES: ReadonlyArray<{
|
||||
name: string;
|
||||
create: string;
|
||||
}> = [
|
||||
{
|
||||
// #348 perf-indexes — pages.title trigram (coalesce-free functional expr).
|
||||
name: 'idx_pages_title_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_pages_title_trgm ' +
|
||||
'ON pages USING gin ((LOWER(f_unaccent(title))) gin_trgm_ops)',
|
||||
},
|
||||
{
|
||||
// #348 perf-indexes — users.name trigram (member search-suggest).
|
||||
name: 'idx_users_name_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_users_name_trgm ' +
|
||||
'ON users USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)',
|
||||
},
|
||||
{
|
||||
// #443 search-lookup-trgm — pages.text_content trigram (the slow, large one).
|
||||
name: 'idx_pages_text_content_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_pages_text_content_trgm ' +
|
||||
'ON pages USING gin ((LOWER(f_unaccent(text_content))) gin_trgm_ops)',
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
* Best-effort, non-transactional pre-build of {@link CONCURRENT_INDEXES}. Run
|
||||
* BEFORE the migrator so the blocking `CREATE INDEX` in the corresponding
|
||||
* migration becomes an `IF NOT EXISTS` no-op.
|
||||
*
|
||||
* `db` MUST be the top-level Kysely instance (NOT a transaction): each statement
|
||||
* then executes on its own connection with no surrounding `BEGIN`, which is
|
||||
* required for `CONCURRENTLY`. Every statement is independent and swallowed on
|
||||
* error (a missing `pages`/`f_unaccent` on a fresh DB, an unsupported driver
|
||||
* path, a permissions gap): the migration backstop still builds the index, so a
|
||||
* failure here is never fatal. `onLog` reports progress/failures for the caller
|
||||
* to route to its logger.
|
||||
*/
|
||||
export async function ensureConcurrentIndexes(
|
||||
db: Kysely<any>,
|
||||
onLog?: (message: string, error?: unknown) => void,
|
||||
): Promise<void> {
|
||||
for (const idx of CONCURRENT_INDEXES) {
|
||||
try {
|
||||
await sql.raw(idx.create).execute(db);
|
||||
onLog?.(`Concurrent index ensured: ${idx.name}`);
|
||||
} catch (error) {
|
||||
// Non-fatal by design — the migration's IF NOT EXISTS create is the
|
||||
// backstop. Common benign cause: the table/function does not exist yet on
|
||||
// a fresh DB (the migrations will build the index instead).
|
||||
onLog?.(
|
||||
`Concurrent index pre-build skipped for ${idx.name} ` +
|
||||
`(will fall back to the in-migration build)`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -33,16 +33,18 @@ import { type Kysely, sql } from 'kysely';
|
||||
* - comments: `findPageComments` does WHERE page_id ORDER BY id ASC, but only
|
||||
* `(page_id)` exists → extra sort.
|
||||
*
|
||||
* DEPLOY-TIME LOCK WARNING: these are plain (non-CONCURRENT) CREATE INDEX
|
||||
* statements — CONCURRENTLY is impossible because Kysely runs each migration in a
|
||||
* transaction. They take a SHARE lock that BLOCKS writes (INSERT/UPDATE/DELETE) on
|
||||
* pages/users/groups/comments/page_history for the duration of the build. The two
|
||||
* GIN trigram builds on pages.title / users.name are the slow ones and can take
|
||||
* minutes on a large tenant → a write-outage window during the deploy migration.
|
||||
* For large installations, run this migration in a maintenance window, or build
|
||||
* the trigram indexes out-of-band with CREATE INDEX CONCURRENTLY before deploying
|
||||
* (then this migration's `IF NOT EXISTS` is a no-op). Small/typical tenants are
|
||||
* unaffected.
|
||||
* DEPLOY-TIME LOCK: these are plain (non-CONCURRENT) CREATE INDEX statements —
|
||||
* CONCURRENTLY is impossible HERE because Kysely runs each migration in a
|
||||
* transaction. The two GIN trigram builds on pages.title / users.name are the
|
||||
* slow ones and would take a SHARE lock that BLOCKS writes on pages/users for
|
||||
* minutes on a large tenant. To avoid that, `ensureConcurrentIndexes`
|
||||
* (database/concurrent-indexes.ts) now pre-builds BOTH trigram indexes with
|
||||
* CREATE INDEX CONCURRENTLY (no transaction) BEFORE the migrator runs, so on an
|
||||
* existing DB the two `IF NOT EXISTS` trigram creates below no-op and no write
|
||||
* lock is taken. On a fresh DB the pre-build is skipped and they build on an
|
||||
* empty table. Keep the two trigram creates in lockstep with their CANONICAL
|
||||
* definitions in CONCURRENT_INDEXES. (The plain b-tree indexes further down are
|
||||
* fast metadata-only builds; they are not pre-built.)
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// Index-compatible, output-identical redefinition of f_unaccent (see header).
|
||||
|
||||
@@ -26,13 +26,16 @@ import { type Kysely, sql } from 'kysely';
|
||||
* to update than b-trees); on the small instances this fork targets that cost
|
||||
* is acceptable and the read win on agent lookups is the priority.
|
||||
*
|
||||
* DEPLOY-TIME LOCK WARNING: plain (non-CONCURRENT) CREATE INDEX — Kysely runs
|
||||
* each migration in a transaction, so CONCURRENTLY is impossible. The build takes
|
||||
* a SHARE lock that BLOCKS writes on `pages` for its duration. The text_content
|
||||
* GIN build is the slow one and can take minutes on a large tenant. For large
|
||||
* installations, run this in a maintenance window or build the index out-of-band
|
||||
* with CREATE INDEX CONCURRENTLY before deploying (then `IF NOT EXISTS` no-ops
|
||||
* here). Small/typical tenants are unaffected.
|
||||
* DEPLOY-TIME LOCK: this is a plain (non-CONCURRENT) CREATE INDEX — Kysely runs
|
||||
* each migration in a transaction, so CONCURRENTLY is impossible HERE, and the
|
||||
* build would take a SHARE lock that BLOCKS writes on `pages` for its duration
|
||||
* (the text_content GIN build can take minutes on a large tenant). To avoid that,
|
||||
* `ensureConcurrentIndexes` (database/concurrent-indexes.ts) now pre-builds this
|
||||
* index with CREATE INDEX CONCURRENTLY (no transaction) BEFORE the migrator runs,
|
||||
* so on an existing DB the `IF NOT EXISTS` below no-ops and no write lock is taken.
|
||||
* On a fresh DB the pre-build is skipped and this builds it on an empty table
|
||||
* where the lock is irrelevant. Keep this create in lockstep with the CANONICAL
|
||||
* definition in CONCURRENT_INDEXES (same expression + opclass).
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// The title predicate is served by #348's idx_pages_title_trgm — see header.
|
||||
|
||||
@@ -4,6 +4,7 @@ import { promises as fs } from 'fs';
|
||||
import { Migrator, FileMigrationProvider } from 'kysely';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
import { ensureConcurrentIndexes } from '@docmost/db/concurrent-indexes';
|
||||
|
||||
@Injectable()
|
||||
export class MigrationService {
|
||||
@@ -12,6 +13,16 @@ export class MigrationService {
|
||||
constructor(@InjectKysely() private readonly db: KyselyDB) {}
|
||||
|
||||
async migrateToLatest(): Promise<void> {
|
||||
// Build write-blocking trigram indexes CONCURRENTLY (no transaction) BEFORE
|
||||
// the migrator runs, so the corresponding in-migration `CREATE INDEX IF NOT
|
||||
// EXISTS` no-ops instead of taking a SHARE lock on `pages` during deploy
|
||||
// (#495). Best-effort: on a fresh DB (no `pages`/`f_unaccent` yet) this is a
|
||||
// no-op and the migrations build the index normally.
|
||||
await ensureConcurrentIndexes(this.db, (message, error) => {
|
||||
if (error) this.logger.warn(`${message}: ${String(error)}`);
|
||||
else this.logger.log(message);
|
||||
});
|
||||
|
||||
const migrator = new Migrator({
|
||||
db: this.db,
|
||||
provider: new FileMigrationProvider({
|
||||
|
||||
Reference in New Issue
Block a user