fix(install): silence clean-install postgres log noise (#484)
Two latent install-time issues that emitted scary postgres ERROR lines on every fresh start but didn't actually break anything. MrGabri flagged them after #494 had already cleared the FK-ordering crash. 1. Migration 035 builds three `CREATE INDEX` statements against `backup_runs(created_at, …)`, but 029 creates the table with `started_at` and no `created_at`. The wrapping try/catch silently swallowed the resulting `column "created_at" does not exist` ERROR, so the migration "succeeded" without ever creating the indexes. Switched 035 to reference `started_at` (same chronological semantics) and added migration 105 to create the same indexes idempotently for deployments whose 035 already ran and silently failed. 2. `run-migrations-safe.js` snapshots `appliedFilenames` *before* `detectExistingSchema()` runs. When `detectExistingSchema()` inserts a row for e.g. `004_add_categories_and_cms.js` (because its tables exist from a partially-completed prior install), the subsequent migration loop still doesn't know about that insert, attempts the legacy migration anyway, and its transaction-internal `insert into migrations` conflicts with the row already there. Re-query the applied set after detectExistingSchema so the loop sees the corrected snapshot. No behavioural change for healthy installs. New installs no longer log the `column "created_at" does not exist` or `duplicate key value violates unique constraint "migrations_filename_unique"` ERRORs.
This commit is contained in:
@@ -83,11 +83,15 @@ async function up() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add indexes if they don't exist
|
// Add indexes if they don't exist. backup_runs (created in 029) tracks
|
||||||
|
// chronology via `started_at` — the original `created_at` reference
|
||||||
|
// here was a bug that emitted a "column does not exist" ERROR in the
|
||||||
|
// postgres log on every fresh install (silently caught below). See
|
||||||
|
// migration 105 for the matching back-fix on already-applied installs.
|
||||||
try {
|
try {
|
||||||
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_mode_status ON backup_runs(backup_mode, status)');
|
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_mode_status ON backup_runs(backup_mode, status)');
|
||||||
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_parent ON backup_runs(parent_backup_id)');
|
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_parent ON backup_runs(parent_backup_id)');
|
||||||
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_created_mode ON backup_runs(created_at, backup_mode)');
|
await db.raw('CREATE INDEX IF NOT EXISTS idx_backup_runs_started_mode ON backup_runs(started_at, backup_mode)');
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.log('Note: Some indexes may already exist, continuing...');
|
console.log('Note: Some indexes may already exist, continuing...');
|
||||||
}
|
}
|
||||||
@@ -144,16 +148,17 @@ async function up() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add composite indexes for common query patterns
|
// Add composite indexes for common query patterns. Same `started_at`
|
||||||
|
// correction as above — `created_at` doesn't exist on backup_runs.
|
||||||
try {
|
try {
|
||||||
await db.raw(`
|
await db.raw(`
|
||||||
CREATE INDEX IF NOT EXISTS idx_backup_runs_recent_successful
|
CREATE INDEX IF NOT EXISTS idx_backup_runs_recent_successful
|
||||||
ON backup_runs(created_at DESC)
|
ON backup_runs(started_at DESC)
|
||||||
WHERE status = 'completed' AND backup_mode = 'full';
|
WHERE status = 'completed' AND backup_mode = 'full';
|
||||||
`);
|
`);
|
||||||
await db.raw(`
|
await db.raw(`
|
||||||
CREATE INDEX IF NOT EXISTS idx_backup_runs_incremental_chain
|
CREATE INDEX IF NOT EXISTS idx_backup_runs_incremental_chain
|
||||||
ON backup_runs(parent_backup_id, created_at)
|
ON backup_runs(parent_backup_id, started_at)
|
||||||
WHERE backup_mode = 'incremental';
|
WHERE backup_mode = 'incremental';
|
||||||
`);
|
`);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -194,10 +199,13 @@ async function down() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Drop indexes
|
// Drop indexes. `idx_backup_runs_created_mode` is the legacy name
|
||||||
|
// shipped by an earlier revision of this migration; kept in the drop
|
||||||
|
// list so a down() against any historic state cleans up either name.
|
||||||
try {
|
try {
|
||||||
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_mode_status');
|
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_mode_status');
|
||||||
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_parent');
|
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_parent');
|
||||||
|
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_started_mode');
|
||||||
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_created_mode');
|
await db.raw('DROP INDEX IF EXISTS idx_backup_runs_created_mode');
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
// Ignore errors if indexes don't exist
|
// Ignore errors if indexes don't exist
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
/**
|
||||||
|
* Migration: back-fix the backup_runs indexes that migration 035 tried to
|
||||||
|
* create on the nonexistent `created_at` column (#484).
|
||||||
|
*
|
||||||
|
* On Postgres, 035's `CREATE INDEX ... ON backup_runs(created_at, ...)`
|
||||||
|
* statements raised `column "created_at" does not exist`, which was caught
|
||||||
|
* silently by the wrapping try/catch — so the migration "succeeded" but the
|
||||||
|
* indexes never got created. Fresh installs saw the ERROR in the postgres
|
||||||
|
* log; existing installs simply ran without those indexes.
|
||||||
|
*
|
||||||
|
* 035 has now been corrected to use `started_at` (the column that does
|
||||||
|
* exist on backup_runs and carries the same chronological semantics).
|
||||||
|
* This migration creates the same indexes idempotently for any deployment
|
||||||
|
* whose 035 silently failed — no-op on fresh installs because 035 already
|
||||||
|
* built them.
|
||||||
|
*
|
||||||
|
* SQLite: partial indexes (`WHERE …`) work but cross-table semantics differ
|
||||||
|
* slightly from Postgres; we still emit them because the only consumer is
|
||||||
|
* the backup-history query in `backupService` and it issues identical SQL
|
||||||
|
* across both backends.
|
||||||
|
*/
|
||||||
|
|
||||||
|
exports.up = async function(knex) {
|
||||||
|
if (!(await knex.schema.hasTable('backup_runs'))) return;
|
||||||
|
if (!(await knex.schema.hasColumn('backup_runs', 'started_at'))) return;
|
||||||
|
|
||||||
|
// Plain composite index — matches the corrected statement in 035.
|
||||||
|
await knex.raw(
|
||||||
|
'CREATE INDEX IF NOT EXISTS idx_backup_runs_started_mode ON backup_runs(started_at, backup_mode)'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Partial indexes only get created if backup_mode exists (035 added it).
|
||||||
|
if (!(await knex.schema.hasColumn('backup_runs', 'backup_mode'))) return;
|
||||||
|
|
||||||
|
await knex.raw(`
|
||||||
|
CREATE INDEX IF NOT EXISTS idx_backup_runs_recent_successful
|
||||||
|
ON backup_runs(started_at DESC)
|
||||||
|
WHERE status = 'completed' AND backup_mode = 'full'
|
||||||
|
`);
|
||||||
|
|
||||||
|
if (await knex.schema.hasColumn('backup_runs', 'parent_backup_id')) {
|
||||||
|
await knex.raw(`
|
||||||
|
CREATE INDEX IF NOT EXISTS idx_backup_runs_incremental_chain
|
||||||
|
ON backup_runs(parent_backup_id, started_at)
|
||||||
|
WHERE backup_mode = 'incremental'
|
||||||
|
`);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
exports.down = async function(knex) {
|
||||||
|
if (!(await knex.schema.hasTable('backup_runs'))) return;
|
||||||
|
await knex.raw('DROP INDEX IF EXISTS idx_backup_runs_started_mode');
|
||||||
|
await knex.raw('DROP INDEX IF EXISTS idx_backup_runs_recent_successful');
|
||||||
|
await knex.raw('DROP INDEX IF EXISTS idx_backup_runs_incremental_chain');
|
||||||
|
};
|
||||||
@@ -126,18 +126,26 @@ async function runMigrations() {
|
|||||||
const hasActivityLogsTable = await db.schema.hasTable('activity_logs');
|
const hasActivityLogsTable = await db.schema.hasTable('activity_logs');
|
||||||
|
|
||||||
// Get applied migrations
|
// Get applied migrations
|
||||||
const appliedMigrations = await db('migrations').select('filename');
|
let appliedMigrations = await db('migrations').select('filename');
|
||||||
const appliedFilenames = appliedMigrations.map(m => m.filename);
|
let appliedFilenames = appliedMigrations.map(m => m.filename);
|
||||||
|
|
||||||
// Check if this is a new deployment
|
// Check if this is a new deployment
|
||||||
// It's new if no essential tables exist OR no migrations have been applied
|
// It's new if no essential tables exist OR no migrations have been applied
|
||||||
const hasEssentialTables = hasEventsTable && hasPhotosTable && hasAdminTable && hasActivityLogsTable;
|
const hasEssentialTables = hasEventsTable && hasPhotosTable && hasAdminTable && hasActivityLogsTable;
|
||||||
const isDatabaseEmpty = !hasEventsTable && !hasPhotosTable && !hasAdminTable && !hasActivityLogsTable;
|
const isDatabaseEmpty = !hasEventsTable && !hasPhotosTable && !hasAdminTable && !hasActivityLogsTable;
|
||||||
const isNewDeployment = isDatabaseEmpty || (appliedFilenames.length === 0 && !hasEssentialTables);
|
const isNewDeployment = isDatabaseEmpty || (appliedFilenames.length === 0 && !hasEssentialTables);
|
||||||
|
|
||||||
// Only detect existing schema for truly existing deployments
|
// Only detect existing schema for truly existing deployments
|
||||||
if (!isNewDeployment) {
|
if (!isNewDeployment) {
|
||||||
await detectExistingSchema();
|
await detectExistingSchema();
|
||||||
|
// detectExistingSchema may have inserted rows into the migrations
|
||||||
|
// table (e.g. for 004_add_categories_and_cms.js when photo_categories
|
||||||
|
// already exists). Re-query so the iteration below sees the up-to-date
|
||||||
|
// applied set — otherwise the loop attempts those migrations again,
|
||||||
|
// their tx-internal `insert into migrations` conflicts, and postgres
|
||||||
|
// logs a "duplicate key" ERROR on every fresh-after-partial install.
|
||||||
|
appliedMigrations = await db('migrations').select('filename');
|
||||||
|
appliedFilenames = appliedMigrations.map(m => m.filename);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get migration files from appropriate directories
|
// Get migration files from appropriate directories
|
||||||
|
|||||||
Reference in New Issue
Block a user