fix: support orioledb migrations by removing concurrent index creation (#673)

* fix: support orioledb migrations by removing concurrent index creation

* enable transformer to fix migrations for oriole db

* remove version from docker compose to stop warning about it being obsolete/ignored

* fix transformers folder naming

Author: itslenny

.docker/docker-compose-infra-oriole-override.yml

@@ -0,0 +1,7 @@
+services:
+  tenant_db:
+    image: orioledb/orioledb:latest-pg17
+    environment:
+      POSTGRES_INITDB_ARGS: "--locale=C"
+    volumes:
+      - ./.docker/init-oriole-db:/docker-entrypoint-initdb.d

.docker/docker-compose-infra.yml

@@ -1,6 +1,5 @@
 # docker-compose.yml
 
-version: '3'
 services:
 
   tenant_db:

.docker/docker-compose-monitoring.yml

@@ -1,4 +1,3 @@
-version: '3'
 services:
   pg_bouncer_exporter:
     image: spreaker/prometheus-pgbouncer-exporter

.docker/init-oriole-db/init-oriole.sql

@@ -0,0 +1,4 @@
+-- Enable oriole extension and set it as the default for table creation
+-- Without running this script oriole db defaults to creating heap tables
+CREATE EXTENSION IF NOT EXISTS orioledb;
+ALTER DATABASE postgres SET default_table_access_method = 'orioledb';

.github/workflows/ci.yml

@@ -82,8 +82,18 @@ jobs:
           S3_PROTOCOL_ACCESS_KEY_ID: ${{ secrets.TENANT_ID }}
           S3_PROTOCOL_ACCESS_KEY_SECRET: ${{ secrets.SERVICE_KEY }}
 
-
       - name: Upload coverage results to Coveralls
         uses: coverallsapp/github-action@master
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Ensure OrioleDB migration compatibility
+        run: |
+          npm run infra:restart:oriole
+        env:
+          PGRST_JWT_SECRET: ${{ secrets.PGRST_JWT_SECRET }}
+          DATABASE_URL: postgresql://postgres:[email protected]/postgres
+          DB_INSTALL_ROLES: true
+          ENABLE_DEFAULT_METRICS: false
+          PG_QUEUE_ENABLE: false
+          MULTI_TENANT: false

package.json

@@ -18,7 +18,9 @@
     "eslint:check": "eslint 'src/**'",
     "infra:stop": "docker compose --project-directory . -f ./.docker/docker-compose-infra.yml down --remove-orphans",
     "infra:start": "docker compose --project-directory . -f ./.docker/docker-compose-infra.yml up -d && sleep 5 && npm run migration:run",
-    "infra:restart": "npm run infra:stop && npm run infra:start"
+    "infra:start:oriole": "docker compose --project-directory . -f ./.docker/docker-compose-infra.yml -f ./.docker/docker-compose-infra-oriole-override.yml up -d && sleep 5 && npm run migration:run",
+    "infra:restart": "npm run infra:stop && npm run infra:start",
+    "infra:restart:oriole": "npm run infra:stop && npm run infra:start:oriole"
   },
   "author": "Supabase",
   "license": "ISC",

src/internal/database/migrations/migrate.ts

@@ -14,6 +14,7 @@ import { ResetMigrationsOnTenant, RunMigrationsOnTenants } from '@storage/events
 import { ERRORS } from '@internal/errors'
 import { DBMigration } from './types'
 import { getSslSettings } from '../util'
+import { MigrationTransformer, DisableConcurrentIndexTransformer } from './transformers'
 
 const {
   multitenantDatabaseUrl,
@@ -224,7 +225,7 @@ export async function obtainLockOnMultitenantDB<T>(fn: () => Promise<T>) {
   } finally {
     try {
       await multitenantKnex.raw(`SELECT pg_advisory_unlock(?);`, ['-8575985245963000605'])
-    } catch (e) {}
+    } catch {}
   }
 }
 
@@ -550,12 +551,15 @@ export async function migrate({
   shouldCreateStorageSchema,
   upToMigration,
 }: MigrateOptions): Promise<Array<Migration>> {
+  const accessMethod = await getDefaultAccessMethod(client)
   return withAdvisoryLock(
     waitForLock,
     runMigrations({
       migrationsDirectory,
       shouldCreateStorageSchema,
       upToMigration,
+      // Remove concurrent index creation if we're using oriole db as it does not support it currently
+      transformers: accessMethod === 'orioledb' ? [new DisableConcurrentIndexTransformer()] : [],
     })
   )(client)
 }
@@ -564,6 +568,7 @@ interface RunMigrationOptions {
   migrationsDirectory: string
   shouldCreateStorageSchema?: boolean
   upToMigration?: keyof typeof DBMigration
+  transformers?: MigrationTransformer[]
 }
 
 /**
@@ -576,6 +581,7 @@ function runMigrations({
   migrationsDirectory,
   shouldCreateStorageSchema,
   upToMigration,
+  transformers = [],
 }: RunMigrationOptions) {
   return async (client: BasicPgClient) => {
     let intendedMigrations = await loadMigrationFilesCached(migrationsDirectory)
@@ -648,14 +654,17 @@ function runMigrations({
       }
 
       for (const migration of migrationsToRun) {
-        const result = await runMigration(migrationTableName, client)(migration)
+        const result = await runMigration(
+          migrationTableName,
+          client
+        )(runMigrationTransformers(migration, transformers))
         completedMigrations.push(result)
       }
 
       return completedMigrations
-    } catch (e: any) {
-      const error: MigrationError = new Error(`Migration failed. Reason: ${e.message}`)
-      error.cause = e
+    } catch (e) {
+      const error: MigrationError = new Error(`Migration failed. Reason: ${(e as Error).message}`)
+      error.cause = e + ''
       throw error
     }
   }
@@ -675,6 +684,30 @@ function filterMigrations(
   return migrations.filter(notAppliedMigration)
 }
 
+/**
+ * Transforms provided migration by running all transformers
+ * @param migration
+ * @param transformers
+ */
+function runMigrationTransformers(
+  migration: Migration,
+  transformers: MigrationTransformer[]
+): Migration {
+  for (const transformer of transformers) {
+    migration = transformer.transform(migration)
+  }
+  return migration
+}
+
+/**
+ * Get the current default access method for this database
+ * @param client
+ */
+async function getDefaultAccessMethod(client: BasicPgClient): Promise<string> {
+  const result = await client.query(`SHOW default_table_access_method`)
+  return result.rows?.[0]?.default_table_access_method || ''
+}
+
 /**
  * Checks if a table exists
  * @param client
@@ -755,7 +788,7 @@ function withAdvisoryLock<T>(
     } finally {
       try {
         await client.query('SELECT pg_advisory_unlock(-8525285245963000605);')
-      } catch (e) {}
+      } catch {}
     }
   }
 }
@@ -828,11 +861,11 @@ async function refreshMigrationPosition(
     migrations.push(intendedMigrations[migration.index])
 
     // add the other run migrations by updating their id and hash
-    const afterMigration = newMigrations.slice(migration.index).map((m) => {
-      ;(m as any).id = m.id + 1
-      ;(m as any).hash = intendedMigrations[m.id].hash
-      return m
-    })
+    const afterMigration = newMigrations.slice(migration.index).map((m) => ({
+      ...m,
+      id: m.id + 1,
+      hash: intendedMigrations[m.id].hash,
+    }))
 
     migrations.push(...afterMigration)
     newMigrations = migrations
@@ -868,18 +901,18 @@ async function refreshMigrationPosition(
  * Memoizes a promise
  * @param func
  */
-function memoizePromise<T, Args extends any[]>(
+function memoizePromise<T, Args extends unknown[]>(
   func: (...args: Args) => Promise<T>
 ): (...args: Args) => Promise<T> {
   const cache = new Map<string, Promise<T>>()
 
   function generateKey(args: Args): string {
     return args
       .map((arg) => {
-        if (typeof arg === 'object') {
+        if (typeof arg === 'object' && arg !== null) {
           return Object.entries(arg).sort().toString()
         }
-        return arg.toString()
+        return String(arg)
       })
       .join('|')
   }

src/internal/database/migrations/transformers/disable-concurrent-index-transformer.ts

@@ -0,0 +1,25 @@
+import { Migration } from 'postgres-migrations/dist/types'
+import { MigrationTransformer } from './transformer'
+
+const CONCURRENT_INDEX_FIND = 'INDEX CONCURRENTLY'
+const CONCURRENT_INDEX_REPLACE = 'INDEX'
+const DISABLE_TRANSACTION_STRING = '-- postgres-migrations disable-transaction'
+
+export class DisableConcurrentIndexTransformer implements MigrationTransformer {
+  transform(migration: Migration): Migration {
+    if (!migration.sql.includes(CONCURRENT_INDEX_FIND)) {
+      return migration
+    }
+
+    return {
+      ...migration,
+      // strip concurrently, and remove disable-transaction in sql and contents
+      sql: migration.sql
+        .replaceAll(CONCURRENT_INDEX_FIND, CONCURRENT_INDEX_REPLACE)
+        .replace(DISABLE_TRANSACTION_STRING, ''),
+      contents: migration.contents
+        .replaceAll(CONCURRENT_INDEX_FIND, CONCURRENT_INDEX_REPLACE)
+        .replace(DISABLE_TRANSACTION_STRING, ''),
+    }
+  }
+}

src/internal/database/migrations/transformers/index.ts

@@ -0,0 +1,2 @@
+export * from './transformer'
+export * from './disable-concurrent-index-transformer'

src/internal/database/migrations/transformers/transformer.ts

@@ -0,0 +1,5 @@
+import { Migration } from 'postgres-migrations/dist/types'
+
+export interface MigrationTransformer {
+  transform(input: Migration): Migration
+}