VahidN / EFSecondLevelCache.Core

Entity Framework Core Second Level Caching Library
Apache License 2.0
326 stars 51 forks source link

.Cacheable() method is locking thread in async environment? #55

Closed ryazanov-dmitry closed 4 years ago

ryazanov-dmitry commented 4 years ago

Summary of the issue

I have multiple EF select queries that run in async manner, bassically with Task.WhenAll using single DbContext object.

Environment

.NET Core SDK version: 2.2
Microsoft.EntityFrameworkCore version: 2.2.6
EFSecondLevelCache.Core version: 2.8

Example code/Steps to reproduce:

var q1 = productTypeUseCase.GetAll();
var q2 = applicationDataUseCase.GetAllApplicationTypes();
var q3 = caseTypeUseCase.GetAll();
await Task.WhenAll(q1, q2, q3);
return true;

## Output:
Application started. Press Ctrl+C to shut down.
[12:26:01 INF] Request starting HTTP/1.1 POST http://localhost:5000/graphql application/json 93
[12:26:01 INF] CORS policy execution successful.
ctx
product type started
app started.
refdata started
app finished. 179
refdata finished 153
product type finished 631
[12:26:03 INF] GraphQL request
[12:26:03 INF] {"query":"{\n  referenceData {\n    testAsync\n  }\n}\n","variables":{},"operationName":null}
[12:26:03 INF] GraphQL response
[12:26:03 INF] Request finished in 1698.1173ms 400 application/json

Now, when i add .Cacheable() to one of queries

##Output:
ctx
product type started
app started.
refdata started
app finished. 244
product type finished 705
[12:32:01 ERR] System.InvalidOperationException: A second operation started on this context before a previous operation completed. This is usually caused by different threads using the same instance of DbContext, however instance members are not guaranteed to be thread safe. This could also be caused by a nested query being evaluated on the client, if this is the case rewrite the query avoiding nested invocations.
   at Microsoft.EntityFrameworkCore.Internal.ConcurrencyDetector.EnterCriticalSection()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)                                         at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at EFSecondLevelCache.Core.EFMaterializer.Materialize[T](Expression expression, Func`1 materializer)
   at EFSecondLevelCache.Core.EFCachedQueryable`1.System.Collections.Generic.IEnumerable<TType>.GetEnumerator()
   at EFSecondLevelCache.Core.EFCachedQueryable`1.get_AsyncEnumerable()
   at Microsoft.EntityFrameworkCore.Extensions.Internal.QueryableExtensions.AsAsyncEnumerable[TSource](IQueryable`1 source)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at .UW.Features.Common.Infrastructure.Repositories.RefDataRepository.GetAll(RefDataMapping refDataMapping) in C:\work\projects\luca\uw-bff\.UW.Features\Common\Infrastructure\Repositories\RefDataRepository.cs:line 110
   at .UW.API.GraphQL.Schema.ReferenceDataType.<>c__DisplayClass0_0.<<-ctor>b__46>d.MoveNext() in C:\work\projects\luca\uw-bff\.UW.API\GraphQL\Schema\ReferenceDataType.cs:line 290
--- End of stack trace from previous location where exception was thrown ---
   at Instrumentation.MiddlewareResolver.Resolve(ResolveFieldContext context)
   at LUCA.UW.API.GraphQL.GraphQLErrorsFieldMiddleware.Resolve(ResolveFieldContext context, FieldMiddlewareDelegate next) in C:\work\projects\luca\uw-bff\LUCA.UW.API\GraphQL\GraphqlErrorsFieldMiddleware.cs:line 24
[12:32:01 INF] GraphQL request
[12:32:01 INF] {"query":"{\n  referenceData {\n    testAsync\n  }\n}\n","variables":{},"operationName":null}
[12:32:01 INF] GraphQL response
VahidN commented 4 years ago

I have improved the concurrency checks in v2.8.0 for async methods using semaphoreSlim. But it seems you are using EFMaterializer.Materializ and not the MaterializeAsync one.

ryazanov-dmitry commented 4 years ago

How i can force code to use .MaterializeAsync instead of .Materialize which is shown in stacktrace?

VahidN commented 4 years ago

Materializing using the ToList method will use the normal synchronous lock, but ToListAsync will use the async semaphoreSlim.WaitAsync version. So just use the xyzAsync methods of EF Core.

ryazanov-dmitry commented 4 years ago

Please check my stacktrace, i am using .ToListAsync()

VahidN commented 4 years ago

What are the details of these 3 methods? I want to test them.

var q1 = productTypeUseCase.GetAll();
var q2 = applicationDataUseCase.GetAllApplicationTypes();
var q3 = caseTypeUseCase.GetAll();
ryazanov-dmitry commented 4 years ago

q1:

public async Task<List<ProductType>> GetAll()
{
    return _uwContext.ProductTypes
        .Select(x =>
        new ProductType
        {
            Code = x.Code,
            Description = x.Description,
            RiskCode = x.RiskCode
        })
        .ToListAsync();
}

q2:

public async Task<List<ApplicationKind>> GetAllApplicationTypes()
{
    return _context.ApplicationTypes.ToListAsync();
}

q3:

public async Task<IList<RefData>> GetAll() 
{
    return await _uwContext.CaseTypes.Select(x=>RefDataMapper.MapRefDataFrom(x)).Cacheable().ToListAsync();
}
VahidN commented 4 years ago

Your sample will crash even without Cacheable() method (I Tested it). Because when q2 (a normal query without any locks) is started, other queries even q1 (another normal query) are not allowed to be started. In EF(6x and Core), you are not allowed to execute multiple queries concurrently using the same context instance (Task.WhenAll = parallel processing = concurrently using the same context instance).

EF Core does not support multiple parallel operations being run on the same context instance. You should always wait for an operation to complete before beginning the next operation.

using concurrent access could affect scalability negatively as it would mean that in order to process a single request you would be spinning of an arbitrary number of different threads. All the threads would compete for resources such as memory with other threads necessary to server other concurrent requests.

EFSecondLevelCache.Core's locking system only applies if all of your 3 queries are using the Cacheable() method.

ryazanov-dmitry commented 4 years ago

Please see this https://stackoverflow.com/a/52857510 And also, why MaterializeAsyncdidn't call in my case?

VahidN commented 4 years ago
lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related problems.