golang-migrate / migrate

Database migrations. CLI and Golang library.
Other
15.32k stars 1.4k forks source link

Neo4j: The whole migrations should run within a single session #716

Open fbiville opened 2 years ago

fbiville commented 2 years ago

Describe the Bug Currently, the Neo4j migration driver opens a session for each Run call (and others such as SetVersion and Version).

This may cause an issue with Neo4j clusters where Neo4j routes a read to a follower that has not necessarily caught up with the leader. In that case, read queries may not return the expected results.

Expected Behavior

To solve this "read your own write" problem, the Neo4j native driver uses bookmarks. They are automatically propagated from the current to to the next operation within the surrounding session. The simplest way is therefore to open a single session in Open and close it when Close is called. Another, more error-prone way, is to explicitly consume bookmarks and pass them to the SessionConfig of the next operation's session.

andrascz commented 2 years ago

I think generically if transactions (or sessions or whatever similar) is supported by a driver (and configured in case of the mongodb driver), then the SetVersion/Run/SetVersion commands inside Migrate.runMigrations should run in a single session/transaction.

fbiville commented 2 years ago

I'm happy to update https://github.com/golang-migrate/migrate/pull/715 accordingly if that's the way to go forward.

andrascz commented 2 years ago

We are hitting a similar issue with mongodb for which the root cause is that while the mongo dockr image runs its initscripts it is accepting external connections. As the service which would use mongo already connects in this phase and runs its migrations, when the init script finishes and mongo restarts the SetVersion with the dirty flag is there. Even though the migration commands are in a transaction, they will fail when mongo shuts down, but the setVersion remains as it is not in the transaction leaving the database in a dirty state, so the next migration fails again because of the dirty flag.

Although this might be possible to solve on the driver level, my proposal provides a better and simpler solution and handles more cases as the Neo4 issue shows.