How to close database in afterMigrate?

[IlyaSemenov]

After each migration, I am doing spawnSync("pg_dump") to store the full schema in a file. I use afterMigrate and afterRollback callbacks for that.

The problem is, if a migration locked some table (that happens with db.changeTable()), pg_dump started in afterMigrate will wait forever for the lock to be released, thus hanging the entire process.

My question is, how do I close db connection in afterMigrate, so that pg_dump will succeed? I tried this:

• await db.close()
• await db.adapter.close()
• await db.adapter.pool.end()

but nothing helped.

[IlyaSemenov]

For the time being, I'm using this workaround:

let migrated = false

export const change = rakeDb(
  { databaseURL },
  {
    async afterMigrate(db) {
      migrated = true
    },
    async afterRollback() {
      migrated = true
    },
  },
)

if (process.env.NODE_ENV === "development" && migrated) {
  change.promise.then(async () => {
    await dumpDatabaseSchema(databaseURL)
  })
}

which is not super elegant and perhaps not very flexible (such as when running migrations programmatically).

[romeerez]

pg_dump started in afterMigrate will wait forever for the lock to be released, thus hanging the entire process.

that's a bug, I'll fix it so migration must release the lock

[romeerez]

I realized it's not a bug, but in fact is a good and protective behavior.

The idea is that the command as a whole runs as a single transaction, and even if an error happens inside a callback - all is rolled back, you can fix the code and try again.

The transaction operates under a lock, so if it's the multi-container environment or something, changes to the same db won't be happening in parallel, but one container will wait for another to finish.

How to solve it? Let's return useful info from the promise, so that you could check what was the command this promise was resolved for.

[romeerez]

Published an update:

• added beforeChage and afterChange for convenience, docs, this won't help you with this use-case
• now the change.promise will return a result object which you can use to check if it was rollback, migrate, etc.

import { execSync } from 'node:child_process';

export const change = rakeDb(
  { databaseURL: 'postgres://...' },
  {
    migrationsPath: 'migrations',
  },
);

change.promise.then(({ options, args: [command] }) => {
  if (process.env.NODE_ENV !== 'development') return;

  if (['up', 'down', 'redo', 'recurrent'].includes(command)) {
    // `as string` is safe because you can see that databaseURL was set above
    dump(options[0].databaseURL as string);
  }
});

function dump(databaseURL: string) {
  execSync(`pg_dump --schema-only ${databaseURL} > structure.sql`);

  console.log('Db structure was dumped to structure.sql');
}

[IlyaSemenov]

That's nice, but seems fragile. For instance, it's not immediately clear which commands to include in the list (whether or not to include 'migrate' depends on whether you deliver a normalized alias or the original command, which is implementation dependent). Anyhow, there's a chance that some new command in future will run migrations and this user-land code will break.

Also, it will run redundantly if there were no migrations.

I agree that both are not a big deal, but what if we return { options, args, changed } to the callback? Like update some calls here:

orchid-orm/packages/rake-db/src/rakeDb.ts

Lines 159 to 168 in 0c67c17

	if (arg === 'create') {
	await createDb(options, config);
	} else if (arg === 'drop') {
	await dropDb(options, config);
	} else if (arg === 'reset') {
	await resetDb(options, config);
	} else if (arg === 'up') {
	await migrate({}, options, config, args.slice(1));
	} else if (arg === 'down') {
	await rollback({}, options, config, args.slice(1));

to return a boolean (or always set, for drop and alike) and then pass it in the return result.

I can come up with a PR if you approve the approach.

[romeerez]

For the first concern, yep, if a new migration command will be added in the future it's fragile. And I already missed that, because reset also performs migrate and I forgot to include it in the docs example.

Let me know if you have ideas for how to solve this. I can think of new callback similar to afterMigrate, but it will be afterChangeCommit: will run both after migrate/rollback and what's important after changes are already saved and the lock is released. Then, the promise result isn't actually needed and can remain without changed status.

Passing changed boolean to the callback is a good way to solve the second concern.

I appreciate your willingness to make PR, but I'm working closely with this area lately and it will be simpler for me to do.

[romeerez]

Done: now there is afterChangeCommit callback. Dumping db example in the docs.

All callbacks now receive migrations array, if it's empty it means nothing was migrated.