-
Notifications
You must be signed in to change notification settings - Fork 280
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
Support for exclusive connection while executing migration #239
Comments
So perhaps I'm missing something, but why not just run the whole set of migrations against such an exclusive connection? That wouldn't require any changes to sql-migrate at all. |
@rubenv as far as I understand the current code uses a connection pool to execute each statement separately and not a common connection across statements (always referring to a single migration file): Line 541 in bc5e259
https://github.com/go-gorp/gorp/blob/2db0f5e22596df067c3d4edf9b2f3e0727cc31ca/gorp.go#L195 https://github.com/go-gorp/gorp/blob/2db0f5e22596df067c3d4edf9b2f3e0727cc31ca/db.go#L34 The conditions that the issue describes can be quite frequent, the The proposed solution sounds to me that would work. If you think there might backwards compatibility issues, perhaps it's best to provide an additional command like |
Context
GitLab's container registry uses
sql-migrate
to manage database migrations. Our database usage has grown to a point where we need to run post-deployment migrations such as creating indexes on partitioned tables. This requires the use ofCONCURRENTLY
in Postgres.We started by creating indexes on just two partitions which worked just fine, with a total runtime of ~30s. We now wanted to execute the post-deployment migrations that will create the remaining partition indexes but it failed due to exceeding the statement timeout limit of 15s that we configure for our Postgres instance.
Problem
We initially thought that we could simply turn off the statement timeout off before executing the statement. However, this won't work for production because with the connection pool on the registry side and PgBouncer's pooling after that, index creation statements and the preceding
SET statement_timeout TO 0
would most likely be executed in different sessions, so the latter wouldn't have an effect on the former.We cannot rely on transactions for these migrations either (as a way to ensure that all statements within would be executed in the same session) because creating an index
CONCURRENTLY
cannot be done within transactions.Solution
The ideal solution in our case, is to simply use an exclusive connection for certain statements rather than relying on the pool abstraction.
At the API level, this could be implemented by adding a new e.g.
ExclusiveConn
parameter to theMigration
struct, similar to DisableTransaction*, which we're already using. If this was set to true on the migration's definition, the lib would grab and use an exclusive connection to execute all its statements.We are happy to make the contribution ourselves, but would like to get a maintainer's opinion on the solution to see if it's something that would be accepted into the project.
Related to https://gitlab.com/gitlab-org/container-registry/-/issues/889.
The text was updated successfully, but these errors were encountered: