Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: How to run migrations outside a transaction #4775

Closed
johansenja opened this issue Oct 2, 2023 · 8 comments
Closed

docs: How to run migrations outside a transaction #4775

johansenja opened this issue Oct 2, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@johansenja
Copy link

I discovered the documentation here which mentions that you can implement an isTransactional() method in your migration to run it non-transactionally:

// my code
export class Migration20231002133126 extends Migration {
  async up(): Promise<void> {
    this.addSql(`CREATE INDEX CONCURRENTLY "my_index" ON my_table(created_at);`);
  }

  async down(): Promise<void> {
    this.addSql(`DROP INDEX "my_index";`);
  }

  isTransactional() {
    return false;
  }
}

But I'm running into a connexion problem when I run this (Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?) - but if I remove the CONCURRENTLY and run it inside a transaction as normal it works perfectly. It makes me wonder if I am missing a step or two here to get it up and running properly?

If so, perhaps it could be helpful to have a more detailed example in the docs? But equally if this is a bug I'm happy to provide a bit more information here

@B4nan
Copy link
Member

B4nan commented Oct 2, 2023

To disable transactions you need to use allOrNothing: false in the config, the isTransactional method is only about the transactions wrapping the migration, but there is still a master transaction over the whole set (as up might execute more than just one migration).

@B4nan
Copy link
Member

B4nan commented Oct 2, 2023

But I'm running into a connexion problem when I run this (Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?)

This is actually a bug, as we need to respect the master transaction even if you disable the per migration transaction, will fix that in a minute. But I believe you still need the allOrNothing: false if your query cannot be executed inside transaction.

@B4nan B4nan added bug Something isn't working and removed documentation labels Oct 2, 2023
@B4nan
Copy link
Member

B4nan commented Oct 2, 2023

And thinking about this more, I guess we could also run such migrations outside of the master transaction right ahead, so you do not need to disable the flag explicitly. The code is already trying to do that, but it timeouts as we only allow single connection in the pool, and we need a second one for such use case.

@B4nan B4nan closed this as completed in e0dfb0c Oct 2, 2023
@johansenja
Copy link
Author

Thanks, using allOrNothing: false solved my problem!

@B4nan
Copy link
Member

B4nan commented Oct 3, 2023

It should actually work without that too with latest version.

@johansenja
Copy link
Author

For what it's worth, I tried updating from 5.6.16 to 5.8.6 and commented out the allOrNothing: false then tried re-ran it just to see, but it still gave that same error listed above

@johansenja
Copy link
Author

I just opened a small PR here in case it's helpful, just mentioning the config option in the isTransactional section! #4778

@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

For what it's worth, I tried updating from 5.6.16 to 5.8.6 and commented out the allOrNothing: false then tried re-ran it just to see, but it still gave that same error listed above

Repro welcome, it does work just fine on my end, tried in the realworld example app.

I just opened a small PR here in case it's helpful, just mentioning the config option in the isTransactional section! #4778

Well, with the changes I made, this is no longer true, so please provide the repro instead. Maybe its something I missed, I was only checking whether it won't get stuck again as it was before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants