elsa-core icon indicating copy to clipboard operation
elsa-core copied to clipboard

The `EntityFrameworkStore.DeleteManyAsync` throws error with database other than SQLite since Elsa 2.9

Open akunzai opened this issue 1 year ago • 18 comments

Hi there, after upgrade to Elsa 2.9.0, some error happens.

  1. Unable Bulk delete workflow instances.
  2. Unable publish workflow definitions.
  3. The CleanupHostedService Failed to perform cleanup.

It may caused by #3286

System.InvalidOperationException: The entity type 'string' was not found. Ensure that the entity type has been added to the model.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.GetOrCreateEntry(Object entity)
   at Microsoft.EntityFrameworkCore.DbContext.EntryWithoutDetectChanges[TEntity](TEntity entity)
   at Microsoft.EntityFrameworkCore.DbContext.Remove[TEntity](TEntity entity)
   at Elsa.Persistence.EntityFramework.Core.Extensions.QueryableBulkExtensions.BatchDeleteWithWorkAroundAsync[T](IQueryable`1 queryable, DbContext elsaContext, CancellationToken cancellationToken)
   at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkWorkflowInstanceStore.<>c__DisplayClass5_0.<<DeleteManyByIdsAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore`2.DoWork(Func`2 work, CancellationToken cancellationToken)
   at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore`2.DoWork(Func`2 work, CancellationToken cancellationToken)
   at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkWorkflowInstanceStore.DeleteManyByIdsAsync(IEnumerable`1 ids, CancellationToken cancellationToken)
   at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkWorkflowInstanceStore.DeleteManyAsync(ISpecification`1 specification, CancellationToken cancellationToken)
   at Elsa.Persistence.Decorators.EventPublishingWorkflowInstanceStore.DeleteManyAsync(ISpecification`1 specification, CancellationToken cancellationToken)
   at Elsa.Retention.Jobs.CleanupJob.DeleteManyAsync(ICollection`1 workflowInstanceIds, CancellationToken cancellationToken)
   at Elsa.Retention.Jobs.CleanupJob.ExecuteAsync(CancellationToken cancellationToken)
   at Elsa.Retention.HostedServices.CleanupHostedService.ExecuteAsync(CancellationToken stoppingToken)

akunzai avatar Oct 06 '22 09:10 akunzai

Hello,

I'll check quickly with a MySQL database.

jdevillard avatar Oct 06 '22 11:10 jdevillard

Can confirm the same issue exists for Postgres (was upgrading from 2.8.2 to 2.9.0):

System.InvalidOperationException The entity type 'string' was not found. Ensure that the entity type has been added to the model. at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.GetOrCreateEntry(Object entity) at Microsoft.EntityFrameworkCore.DbContext.EntryWithoutDetectChanges[TEntity](TEntity entity) at Microsoft.EntityFrameworkCore.DbContext.Remove[TEntity](TEntity entity) at Elsa.Persistence.EntityFramework.Core.Extensions.QueryableBulkExtensions.BatchDeleteWithWorkAroundAsync[T](IQueryable1 queryable, DbContext elsaContext, CancellationToken cancellationToken) at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore2.<>c__DisplayClass11_0.<<DeleteManyAsync>b__0>d.MoveNext() --- End of stack trace from previous location --- at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore2.DoWork[TResult](Func2 work, CancellationToken cancellationToken) at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore2.DoWork[TResult](Func2 work, CancellationToken cancellationToken) at Elsa.Persistence.EntityFramework.Core.Stores.EntityFrameworkStore2.DeleteManyAsync(ISpecification1 specification, CancellationToken cancellationToken) at Elsa.Services.Triggers.TriggerIndexer.DeleteTriggersAsync(String workflowDefinitionId, CancellationToken cancellationToken) at Elsa.Services.Triggers.TriggerIndexer.IndexTriggersAsync(IWorkflowBlueprint workflowBlueprint, CancellationToken cancellationToken) at Elsa.Services.Triggers.TriggerIndexer.IndexTriggersAsync(IEnumerable1 workflowBlueprints, CancellationToken cancellationToken) at Elsa.Services.Triggers.TriggerIndexer.IndexTriggersAsync(CancellationToken cancellationToken) at Elsa.StartupTasks.IndexTriggers.ExecuteAsync(CancellationToken cancellationToken) at Elsa.Runtime.StartupRunner.StartupAsync(CancellationToken cancellationToken) at Elsa.Runtime.StartupRunner.StartupAsync(CancellationToken cancellationToken) at Elsa.HostedServices.StartupRunnerHostedService.StartAsync(CancellationToken cancellationToken) at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken) at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Start(IHost host) at Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory1.CreateHost(IHostBuilder builder)

hf-kklein avatar Oct 06 '22 13:10 hf-kklein

ok, I think I've seen the issue.

I'll try to propose a fix tomorrow.

jdevillard avatar Oct 06 '22 22:10 jdevillard

Getting same issue with MySql.

johnwc avatar Oct 10 '22 04:10 johnwc

@jdevillard I pushed a change that seems to fix this issue, but I haven't been able to test with SQL Server, MySQL Postgres or Oracle as of yet.

@johnwc If you have a chance, could you try the latest 2.10.0.463 preview package (currently being built) and let me know if my change fixes things for MySQL?

The build: https://ci.appveyor.com/project/sfmskywalker/elsa/builds/45031419

sfmskywalker avatar Oct 11 '22 10:10 sfmskywalker

Hello @sfmskywalker ,

during my test , I've seen that issue is due to the select(x=>x.Id) in

           await dbContext.Set<WorkflowExecutionLogRecord>().AsQueryable().Where(x => idList.Contains(x.WorkflowInstanceId)).Select(x=>x.Id).AsQueryable().BatchDeleteWithWorkAroundAsync(dbContext, cancellationToken);
                await dbContext.Set<Bookmark>().AsQueryable().Where(x => idList.Contains(x.WorkflowInstanceId)).Select(x => x.Id).AsQueryable().BatchDeleteWithWorkAroundAsync(dbContext, cancellationToken);
                await dbContext.Set<WorkflowInstance>().AsQueryable().Where(x => idList.Contains(x.Id)).Select(x => x.Id).AsQueryable().BatchDeleteWithWorkAroundAsync(dbContext, cancellationToken);

and also due to the BulkExtensions.

I'm sorry about that bug, but I've seen that , (for .Net6) :

  • BulkExtensions can be updated to 6.5.6 and Workaround for PostGre can be delete
  • BulkExtensions need a PR (I'll submit one) and Workaround for MySql could be deleted also

there is only Oracle that then need the workaround.

jdevillard avatar Oct 11 '22 12:10 jdevillard

Hi @sfmskywalker,

After trying the latest master commit (e205125) with Elsa.Samples.Server.Host project instead of the 2.10.0.463 preview package.

your changes doesn't fix the issue, It throws error during IndexTriggers startup on MySQL.

Failed executing DbCommand (100ms) [Parameters=[@p0='?' (Size = 4000)], CommandType='Text', CommandTimeout='30']
      DELETE FROM Elsa.Triggers WHERE `t`.`WorkflowDefinitionId` = @p0
Unhandled exception. MySqlConnector.MySqlException (0x80004005): Unknown column 't.WorkflowDefinitionId' in 'where clause'
   at MySqlConnector.Core.ServerSession.ReceiveReplyAsyncAwaited(ValueTask`1 task) in /_/src/MySqlConnector/Core/ServerSession.cs:line 908
   at MySqlConnector.Core.ResultSet.ReadResultSetHeaderAsync(IOBehavior ioBehavior) in /_/src/MySqlConnector/Core/ResultSet.cs:line 44
   at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 126
   at MySqlConnector.MySqlDataReader.CreateAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 450
   at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(IReadOnlyList`1 commands, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 56
   at MySqlConnector.MySqlCommand.ExecuteNonQueryAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 264
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.ExecuteSqlRawAsync(DatabaseFacade databaseFacade, String sql, IEnumerable`1 parameters, CancellationToken cancellationToken)

akunzai avatar Oct 11 '22 13:10 akunzai

@jdevillard No worries, thank you for being on top of it.

Let's indeed upgrade to the latest version of BulkExtensions and remove the workarounds that are no longer necessary. Still, it seems to me that for the Oracle provider we would need the workaround, in which case the .Select(x => x.Id) piece would be problematic for the Oracle workaround.

In my latest commit, I replaced the implementations for bulk-deletion by using SQL syntax directly - perhaps we can do this just for the Oracle provider.

Let me know what you have in mind.

Thanks!

sfmskywalker avatar Oct 12 '22 09:10 sfmskywalker

@akunzai I think I see why - notice the `t` in the query - that should have been removed, but in my replacement logic, I only check for double-quotes. Very flaky apparently.

sfmskywalker avatar Oct 12 '22 09:10 sfmskywalker

@akunzai I pushed another change that fixes this when using MySql - the build is pending, but please give it a try when it's finished when you can.

sfmskywalker avatar Oct 12 '22 10:10 sfmskywalker

@jdevillard It looks like we may not be able to take advantage of the BulkExtensions improvements because the latest version takes a dependency on EF Core 6.x, which depends on .NET 6. This would mean that users on .NET 5 will still experience the issue when deleting records. Given that we also need a workaround still for Oracle, regardless of BulkExtensions, maybe we just need to use raw SQL for bulk-deletion like I did (and after we're sure it works on all supported relational databases).

Let me know what you think!

sfmskywalker avatar Oct 12 '22 10:10 sfmskywalker

@sfmskywalker Can't we use multi-targeting for BulkExtensions to resolve this? I remember they had different version for different framework version.

Also officially .NET 5 is out of support since May. I know it is not easy for everyone to switch but could we make use of that to set some cutoff version for .NET 5 support?

mohdali avatar Oct 12 '22 10:10 mohdali

Does #3339 solve this?

nickbeau avatar Oct 12 '22 10:10 nickbeau

@mohdali For the package references yes, but then we also need to use compiler directives to dynamically use the bulk extensions for just Oracle when using .NET 6, and for MySql and Postgres when using .NET 5 or lower. Additionally, since Oracle isn't supported in either version, we still have to have a workaround for that. Which, as far as I can tell, means we need to use raw sql. Which again we could do conditionally using compiler directives. But then I wonder if it's worth that effort over using raw SQL regardless of the DB provider.

That being said, since the multi-targeting is already been done by the pr #3339 as mentioned by @nickbeau to allow them to use EF Core 7 anyway, we can go with that. For .NET < 7, the older BulkExtensions version is used, but when you use .NET 7, the latest version is used, but from what I can tell, no support for Oracle or MySQL - probably because EF Core 7 doesn't support this yet (I presume).

sfmskywalker avatar Oct 12 '22 10:10 sfmskywalker

Hi @sfmskywalker,

I can confirm the latest branch 2.9.x (commit f9ac827) fixes this on MySQL, and I fired another PR #3343 to improve your fixes.

akunzai avatar Oct 12 '22 15:10 akunzai

When do we expect this fix to be pushed to nuget?

johnwc avatar Oct 13 '22 05:10 johnwc

@sfmskywalker With the latest preview from myget, we getting the following error. Our database is not named Elsa, is the Elsa. a accidental missed hardcoded database name in the query?

Microsoft.EntityFrameworkCore.Database.Command[20102] Failed executing DbCommand (0ms) [Parameters=[@p0='?' (Size = 4000)], CommandType='Text', CommandTimeout='30'] DELETE FROM Elsa.Triggers WHERE ``WorkflowDefinitionId`` = @p0

Unhandled exception. MySqlConnector.MySqlException (0x80004005): DELETE command denied to user 'elsa_workflow'@'rt1-worker3-221cd339-4qchr.k8s.local.net' for table 'Triggers'

johnwc avatar Oct 14 '22 02:10 johnwc

@sfmskywalker With the latest preview from myget, we getting the following error. Our database is not named Elsa, is the Elsa. a accidental missed hardcoded database name in the query?

Microsoft.EntityFrameworkCore.Database.Command[20102] Failed executing DbCommand (0ms) [Parameters=[@p0='?' (Size = 4000)], CommandType='Text', CommandTimeout='30'] DELETE FROM Elsa.Triggers WHERE ``WorkflowDefinitionId`` = @p0

Unhandled exception. MySqlConnector.MySqlException (0x80004005): DELETE command denied to user 'elsa_workflow'@'rt1-worker3-221cd339-4qchr.k8s.local.net' for table 'Triggers'

Hello , I think this should be fix with #3345. Indeed , I've seen during my test the issue when we don't use the default database name. And because the notion of schéma is not the same for MySQL , we can remove the schéma name from dbcontext when we use MySQL to avoid this issue.

jdevillard avatar Oct 14 '22 05:10 jdevillard