Skip to content

Implement database locking for migrations #33731

New issue

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

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

Already on GitHub? # to your account

Closed
Tracked by #19587
AndriySvyryd opened this issue May 16, 2024 · 5 comments · Fixed by #34115
Closed
Tracked by #19587

Implement database locking for migrations #33731

AndriySvyryd opened this issue May 16, 2024 · 5 comments · Fixed by #34115

Comments

@AndriySvyryd
Copy link
Member

AndriySvyryd commented May 16, 2024

Generally speaking, concurrent invocations of Migrate() will fail and possibly leave the database in a corrupted state. In order to mitigate this, we could create a temporary table using HistoryRepository that will serve as a lock while the migrations are applied by the process that acquired it. Providers will be able to override all related logic to instead leverage database-specific mechanisms to accomplish this.

@roji
Copy link
Member

roji commented May 16, 2024

That's a good idea - databases do frequently expose locking mechanisms that could work well, e.g. full-table exclusive lock on the HistoryRepository table for PG and SQL Server. Though I'm not sure what a default implementation would look like here - what do you have in mind with the temporary table?

@AndriySvyryd
Copy link
Member Author

what do you have in mind with the temporary table?

Before applying migrations create __EFLock table. If it was created successfully - the lock was acquired. If not, enter the normal retry loop.
After migrations and seeding (#17568) finished applying delete __EFLock table.

@roji
Copy link
Member

roji commented Jul 12, 2024

PG tracking issue: npgsql/efcore.pg#3223

@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview7 Aug 21, 2024
@roji roji removed this from the 9.0.0-preview7 milestone Oct 12, 2024
@roji roji added this to the 9.0.0 milestone Oct 12, 2024
@pisc-software
Copy link

This new feature creates issues when using CrateDB and the NPGSQL implementation it supports. The database does not support the "LOCK" command, hence migrations fail when using Migrate/MigrateAsync with this setup.

The migration is not executed, because the lock on the history table creates an exception in the Postgres driver.

How to tell EF to not perform this table lock?

We are heavily using EF in a large software, and now we are not able to upgrade to .NET 9 because of this.
Whenever adding such a feature, it MUST be such to allow disabling it from code. Not all Databases support LOCK. The assumption made here is incorrect.

@roji
Copy link
Member

roji commented Dec 2, 2024

@pisc-software

We are heavily using EF in a large software, and now we are not able to upgrade to .NET 9 because of this.
Whenever adding such a feature, it MUST be such to allow disabling it from code. Not all Databases support LOCK. The assumption made here is incorrect.

While I'm sorry you're running into trouble, keep in mind that the PostgreSQL EF provider was developed and gets tested against PostgreSQL; it's the responsibility of the various PG-like database (like CrateDB - there are many others) to ensure compatibility. In any case, users shouldn't have to be concerned with controlling a low-level implementation detail such as enabling/disabling migration locking - at most they should configure the EF provider for CrateDB (or similar), and that would determine whether locking is used or not.

In any case, here's a code snippet that you can drop into your project to disable locking with the PostgreSQL EF provider - this should unblock you and anyone else; we'll discuss how to better address the situation in the team.


Workaround to disable migration locking with Npgsql

When configuring your DbContext, add ReplaceService() as follows:

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    => optionsBuilder
        .UseNpgsql("Host=localhost;Username=test;Password=test")
        .ReplaceService<IHistoryRepository, NonLockingNpgsqlHistoryRepository>();

The implementation for NonLockingNpgsqlHistoryRepository:

#pragma warning disable EF1001
public class NonLockingNpgsqlHistoryRepository(HistoryRepositoryDependencies dependencies)
    : NpgsqlHistoryRepository(dependencies)
{
    public override IMigrationsDatabaseLock AcquireDatabaseLock()
        => new NoopMigrationsDatabaseLock(this);

    public override Task<IMigrationsDatabaseLock> AcquireDatabaseLockAsync(
        CancellationToken cancellationToken = default)
        => Task.FromResult<IMigrationsDatabaseLock>(new NoopMigrationsDatabaseLock(this));

    class NoopMigrationsDatabaseLock(NpgsqlHistoryRepository historyRepository) : IMigrationsDatabaseLock
    {
        public void Dispose() {}
        public ValueTask DisposeAsync() => ValueTask.CompletedTask;
        public IHistoryRepository HistoryRepository => historyRepository;
    }
}
#pragma warning restore EF1001

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants