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

Concurrently running migrations - where is locking? #9736

Closed
peterdeme opened this issue Sep 8, 2017 · 4 comments
Closed

Concurrently running migrations - where is locking? #9736

peterdeme opened this issue Sep 8, 2017 · 4 comments

Comments

@peterdeme
Copy link

peterdeme commented Sep 8, 2017

Environment: ASP.NET Core 1.1.2, EF Core 1.1.2, Pomelo.MySQL provider, AWS Container Services + AWS Aurora (MySQL engine)

Hello Guys,

Reason I'm opening this case, because we literally didn't find any similar issues in Google which is weird.

We have a problem with running migrations during application startup. So we have multiple app nodes in our environments, and after we deploy new code and the apps are coming up (at the same time) we see concurrency issues with the migration part. Many times we saw errors due to concurrency problems (ie. "Column already exists"). Actually, I don't understand how come no one ever faced this issue?

My advice would be to implement locking in the database. Something like

DECLARE @isInProgress BIT = SELECT InProgress FROM __EFMigrationHistory WHERE MigrationId = '20170908_AddNewColumn'
DECLARE @finished BIT = SELECT Finished FROM __EFMigrationHistory WHERE MigrationId = '20170908_AddNewColumn'

IF @isInProgress = 0 AND @finished = 0
BEGIN
      UPDATE __EFMigrationHistory SET InProgress = 1 WHERE MigrationId = '20170908_AddNewColumn'

      -- run migration code ie: ALTER TABLE

      UPDATE __EFMigrationHistory SET InProgress = 0 WHERE MigrationId = '20170908_AddNewColumn'
      UPDATE __EFMigrationHistory SET Finished = 1 WHERE MigrationId = '20170908_AddNewColumn'
END   
ELSE
BEGIN
      -- there were race conditions, this migration was already applied
      RETURN
END

(Or whatever your guys idea is)

Right now, probably we will implement distributed lock in Redis to avoid this situation.

Thanks,
Peter

@ajcvickers
Copy link
Member

@peterdeme Running migrations as part of application startup is not something we recommend because of both this kind of concurrency issue and the need for permissions that may not be appropriate for the normally running application. We would recommend that database migration happen as part of application deployment, rather than first run. The deployment may use scripts (e.g. generated from migrations) or may run tools directly against the production database. In either case, it is done in a controlled manner to avoid concurrency and in an environment with appropriate permissions.

@peterdeme
Copy link
Author

peterdeme commented Sep 8, 2017

@ajcvickers
Thanks for the response. I see your point.

"concurrency issue and the need for permissions"

Well, the first issue can be easily(?) solved with a locking method. The second one is not, obviously.

"We would recommend that database migration happen as part of application deployment"

Well, that was my another idea, too. But there can be problems if you run the migration before the app code is updated or after it is updated as well. Before updated: DB schema is already updated but your code is not prepared for that (ie.: column rename). After updated: your code is already prepared for the schema change (ie.: column rename again), but the DB is not updated yet. In both cases, exceptions are thrown. But for sure, that solves the permission problem. Maybe we'll stick with this or the Redis lock, we'll see.

If this won't be implemented, please close this case.

Thanks again.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@caleblloyd
Copy link

Looks like it is being added in 9.0! Ref: #33731

@roji
Copy link
Member

roji commented Sep 12, 2024

Duplicate of #33731

@roji roji marked this as a duplicate of #33731 Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants