win7user10 / Laraue.EfCoreTriggers

Library to write triggers in C# with EF.Core
MIT License
114 stars 22 forks source link

ConvertTriggerAnnotationsToSql can't execute at the same time for a cached model #88

Open JustusGreiberORGADATA opened 1 year ago

JustusGreiberORGADATA commented 1 year ago

Hi,

we are using parallelized the test execution to run our integration tests and therefore use multiple DB Contexts in different threads.

I noticed that the build sometimes fails with the following exception:

Failed Execute_HasPersonsLinked_SendsEmails [5 s]
  Error Message:
   Initialization method Ofcas.Odid.IntegrationTests.InfrastructureTest.QuartzTest.JobsTest.NotifyAboutPrivacyPolicyUpdateJobTest.TestInitialize threw exception. System.InvalidOperationException: Collection was modified after the enumerator was instantiated..
  Stack Trace:
      at System.Collections.Generic.SortedSet`1.Enumerator.MoveNext()
   at System.Collections.Generic.SortedDictionary`2.Enumerator.MoveNext()
   at System.Collections.Generic.SortedDictionary`2.ValueCollection.Enumerator.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at Laraue.EfCoreTriggers.Common.Migrations.MigrationsModelDiffer.GetDifferences(IRelationalModel source, IRelationalModel target)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabaseCreator.GetCreateTablesCommands(MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabaseCreator.CreateTables()
   at Ofcas.Odid.IntegrationTests.SQLiteConnectionTestHelper.TestInitialize()

The failure is more likely the more parallel runners I add.

I investigated this and I think I found the problem. The library modifies the annotations of the model in the MigrationsModelDiffer: https://github.com/win7user10/Laraue.EfCoreTriggers/blob/77b411e0ffc14f13bb118999850e040f15b1befd/src/Laraue.EfCoreTriggers.Common/Migrations/MigrationsModelDiffer.cs#L164-L195

This causes the SortedDict for the annotations to be modified: https://github.com/dotnet/efcore/blob/ab0509d03fd93aecfe692348e2867257a5568d82/src/EFCore/Infrastructure/AnnotatableBase.cs#L24

The issue is caused by the fact, that EF Core build the model only once and caches the it for future calls: https://learn.microsoft.com/en-us/ef/core/modeling/dynamic-model. This means if CreateTables is called by multiple threads at the same time, one thread will modify the annotations, while the other threads holds an iterator for the annotations, by executing GetTriggerAnnotations() here:

https://github.com/win7user10/Laraue.EfCoreTriggers/blob/77b411e0ffc14f13bb118999850e040f15b1befd/src/Laraue.EfCoreTriggers.Common/Migrations/MigrationsModelDiffer.cs#L70

I verified this by creating my own IModelCacheKeyFactory that essentially disables caching by generating a random cache key:

public class NoChachingModelCacheKeyFactory : IModelCacheKeyFactory
{
    public object Create(DbContext context, bool designTime)
        => Guid.NewGuid();
}
new DbContextOptionsBuilder<DbContext>()
                .UseSqlite(_connection)
                .UseSqlLiteTriggers(services => services.AddMethodCallConverter<TriggerMethodCallConverter>())
                .ReplaceService<IModelCacheKeyFactory, NoChachingModelCacheKeyFactory>()
                .Options;

This prevents the exception, but it is obviously not optimal, because it incurs a performance penalty.

Maybe there is a better way to solve this.