Skip to content

Commit 0a3c08a

Browse files
committed
fix(knowledge): retry the Tin projection after a database refuses the extension
A script migration can now defer: `up` throws `ScriptMigrationDeferred`, the runner logs it, leaves the name unrecorded and runs the remaining migrations. The Tin projection defers when the database offers `tin` but refuses to create it, so the upgrade after the extension is permitted installs it instead of skipping it forever.
1 parent 6d5f280 commit 0a3c08a

6 files changed

Lines changed: 119 additions & 8 deletions

File tree

‎apps/sim/lib/knowledge/search/tin-keyword.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ const indexReadiness = new LRUCache<'index', boolean, SearchBudget | undefined>(
3333
},
3434
})
3535

36-
/** Only organization search indexes are projected; `is_search_index` is fixed at creation. */
36+
/**
37+
* Only organization search indexes are projected. `is_search_index` is only ever turned on, when a
38+
* legacy base is adopted, so a stale answer just keeps that base on the GIN projection for one TTL.
39+
*/
3740
const searchIndexBases = new LRUCache<string, boolean>({
3841
max: 10_000,
3942
ttl: SEARCH_INDEX_TTL_MS,
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { Sql } from 'postgres'
5+
import { describe, expect, it, vi } from 'vitest'
6+
import { installTinKeywordProjection } from './0019_tin_keyword_projection'
7+
import { ScriptMigrationDeferred } from './types'
8+
9+
/** A session that offers `tin` or not, and answers `CREATE EXTENSION` with `createError`. */
10+
function createSqlHarness(options: { available: boolean; createError?: { code: string } }) {
11+
const statements: string[] = []
12+
const run = (strings: TemplateStringsArray) => {
13+
const text = strings.join('?').replace(/\s+/g, ' ').trim()
14+
statements.push(text)
15+
if (text.includes('pg_available_extensions')) {
16+
return Promise.resolve(options.available ? [{ '?column?': 1 }] : [])
17+
}
18+
return Promise.resolve([])
19+
}
20+
const sql = run as unknown as Sql
21+
sql.unsafe = vi.fn(async (text: string) => {
22+
statements.push(text)
23+
if (text.startsWith('CREATE EXTENSION') && options.createError) throw options.createError
24+
return []
25+
}) as unknown as Sql['unsafe']
26+
return { sql, statements }
27+
}
28+
29+
describe('installTinKeywordProjection', () => {
30+
it('records a no-op where the database does not offer tin', async () => {
31+
const { sql, statements } = createSqlHarness({ available: false })
32+
expect(await installTinKeywordProjection(sql)).toBeUndefined()
33+
expect(statements.some((text) => text.startsWith('CREATE EXTENSION'))).toBe(false)
34+
})
35+
36+
it.each(['42501', '0A000'])(
37+
'defers without installing anything when the database refuses the extension (%s)',
38+
async (code) => {
39+
const { sql, statements } = createSqlHarness({ available: true, createError: { code } })
40+
await expect(installTinKeywordProjection(sql)).rejects.toBeInstanceOf(ScriptMigrationDeferred)
41+
expect(statements.at(-1)).toBe('CREATE EXTENSION IF NOT EXISTS tin')
42+
}
43+
)
44+
45+
it('fails the migration on any other extension error', async () => {
46+
const { sql } = createSqlHarness({ available: true, createError: { code: '53100' } })
47+
await expect(installTinKeywordProjection(sql)).rejects.toEqual({ code: '53100' })
48+
})
49+
})

‎packages/db/script-migrations/0019_tin_keyword_projection.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { EMBEDDING_KEYWORD_TIN_INDEX } from '@sim/db/schema'
2-
import type { ScriptMigration } from '@sim/db/script-migrations/types'
2+
import { type ScriptMigration, ScriptMigrationDeferred } from '@sim/db/script-migrations/types'
33
import { createLogger } from '@sim/logger'
44
import postgres, { type Sql } from 'postgres'
55

@@ -12,7 +12,8 @@ const EXTENSION_REFUSED_CODES = new Set(['42501', '0A000'])
1212
/**
1313
* Installs the extension, or reports that this database refuses it: listed as available is not
1414
* the same as creatable by the migration role. Tin is an optimization, so a refusal leaves keyword
15-
* search on the GIN projection instead of failing the deploy.
15+
* search on the GIN projection instead of failing the deploy, and defers the migration so the
16+
* upgrade after the extension is allowed installs it.
1617
*/
1718
async function createTinExtension(sql: Sql): Promise<boolean> {
1819
try {
@@ -209,15 +210,18 @@ async function buildProjectionIndex(sql: Sql): Promise<void> {
209210

210211
/**
211212
* Installs and fills the Tin keyword projection where the database offers `tin`, and records a
212-
* no-op elsewhere. A database that gains the extension later runs this file directly (see below)
213-
* to adopt it; the whole migration is idempotent.
213+
* no-op elsewhere. A database that refuses the extension it offers defers instead, so a later
214+
* upgrade retries it. A database that gains the extension after recording the no-op runs this file
215+
* directly (see below) to adopt it; the whole migration is idempotent.
214216
*/
215217
export async function installTinKeywordProjection(sql: Sql): Promise<void> {
216218
if (!(await tinAvailable(sql))) {
217219
logger.info('Tin is unavailable; keyword search keeps the GIN projection')
218220
return
219221
}
220-
if (!(await createTinExtension(sql))) return
222+
if (!(await createTinExtension(sql))) {
223+
throw new ScriptMigrationDeferred('the database refused the tin extension')
224+
}
221225
await installProjection(sql)
222226
const rows = await backfillProjection(sql)
223227
await buildProjectionIndex(sql)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { Sql } from 'postgres'
5+
import { describe, expect, it, vi } from 'vitest'
6+
import { runScriptMigrations, scriptMigrations } from './index'
7+
8+
const TIN = '0019_tin_keyword_projection'
9+
10+
/** A session where every migration but Tin's is recorded and the database refuses `tin`. */
11+
function createSqlHarness() {
12+
const recorded: string[] = []
13+
const run = (strings: TemplateStringsArray, ...values: unknown[]) => {
14+
const text = strings.join('?').replace(/\s+/g, ' ').trim()
15+
if (text.startsWith('SELECT name FROM script_migrations')) {
16+
return Promise.resolve(
17+
scriptMigrations.filter(({ name }) => name !== TIN).map(({ name }) => ({ name }))
18+
)
19+
}
20+
if (text.includes('pg_available_extensions')) return Promise.resolve([{ '?column?': 1 }])
21+
if (text.startsWith('INSERT INTO script_migrations')) recorded.push(values[0] as string)
22+
return Promise.resolve([])
23+
}
24+
const sql = run as unknown as Sql
25+
sql.unsafe = vi.fn(async (text: string) => {
26+
if (text.startsWith('CREATE EXTENSION')) throw { code: '42501' }
27+
return []
28+
}) as unknown as Sql['unsafe']
29+
sql.begin = vi.fn(async (callback) => (callback as (tx: Sql) => unknown)(sql)) as Sql['begin']
30+
return { sql, recorded }
31+
}
32+
33+
describe('runScriptMigrations', () => {
34+
it('leaves a deferred migration unrecorded without failing the upgrade', async () => {
35+
const { sql, recorded } = createSqlHarness()
36+
await expect(runScriptMigrations(sql)).resolves.toBeUndefined()
37+
expect(recorded).toEqual([])
38+
})
39+
})

‎packages/db/script-migrations/index.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { repairUnknownTableRowProvenanceSecondPass } from './0006_repair_unknown
1515
import { repairUnknownWorkspaceFileProvenance } from './0007_repair_unknown_workspace_file_provenance'
1616
import { backfillCredentialGroupResourcePolicies } from './0010_backfill_credential_group_resource_policies'
1717
import { remapLegacyKnowledgeConnectorCredentialsMigration } from './0011_remap_legacy_knowledge_connector_credentials'
18-
import type { ScriptMigration } from './types'
18+
import { type ScriptMigration, ScriptMigrationDeferred } from './types'
1919

2020
export type { ScriptMigration } from './types'
2121

@@ -54,6 +54,7 @@ export const scriptMigrations: readonly ScriptMigration[] = [
5454
*
5555
* Fails fast: a missing required env var or a throwing `up` aborts the run
5656
* before the name is recorded, so the migration retries on the next upgrade.
57+
* A deferred `up` is not recorded either, but lets the later migrations run.
5758
*/
5859
export async function runScriptMigrations(sql: Sql): Promise<void> {
5960
const names = new Set<string>()
@@ -101,7 +102,13 @@ export async function runScriptMigrations(sql: Sql): Promise<void> {
101102
}
102103
console.log(`Applying script migration ${migration.name}...`)
103104
const startedAt = Date.now()
104-
await migration.up(sql)
105+
try {
106+
await migration.up(sql)
107+
} catch (error) {
108+
if (!(error instanceof ScriptMigrationDeferred)) throw error
109+
console.log(`Script migration ${migration.name} deferred: ${error.message}`)
110+
continue
111+
}
105112
await sql.begin(async (tx) => {
106113
for (const name of [migration.name, ...(migration.supersedes ?? [])]) {
107114
await tx`INSERT INTO script_migrations (name) VALUES (${name}) ON CONFLICT (name) DO NOTHING`

‎packages/db/script-migrations/types.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,12 @@ export interface ScriptMigration {
3535
*/
3636
up(sql: Sql): Promise<void>
3737
}
38+
39+
/**
40+
* Thrown by `up` to leave the migration unrecorded without failing the upgrade,
41+
* so the next upgrade runs it again: for work this database refuses today but
42+
* may accept later, such as an extension the migration role may not yet create.
43+
*/
44+
export class ScriptMigrationDeferred extends Error {
45+
override name = 'ScriptMigrationDeferred'
46+
}

0 commit comments

Comments
 (0)