When using a scoped DbContext with multiple DataLoaders all resolving at the same time, I get the following error:
GraphQL.ExecutionError: Error trying to resolve agent. ---> System.InvalidOperationException: A second operation started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.\n at Microsoft.EntityFrameworkCore.Internal.ConcurrencyDetector.EnterCriticalSection()\n...
I can't find any documentation or issues directly related to this, which leads me to believe that I'm using it wrong.
One obvious solution is to change my service registration for the DbContext to Transient, instead of scoped (default), but this will require a large refactor and I have seen a few references that all strongly discourage this.
Any ideas?
It makes perfect sense considering the implemented (and desired) behaviour as demonstrated here:
https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/DataLoader/DataLoaderContext.cs#L52
Also, obviously what one uses the DataLoaders for is ones own concern and out of the domain of the library. Just wondering if there is something obvious I'm missing.
Unfortunately, DbContext is not capable of doing multiple requests at the same time. Using transient DbContext is an option, but you have to carefully dispose of them later, and usage of DbContextFactory may be broken.
You may also try creating a custom context in your resolve methods (or somewhere in you dataloader methods) and use it to work with DbContext.
Field<UserType>(
"getUser",
arguments: new QueryArguments(
new QueryArgument<NonNullGraphType<StringGraphType>> {Name = "name"}
),
resolve: context =>
{
var name = context.GetArgument("name", "");
using (var scope = _serviceProvider.CreateScope())
{
var dbContext = scope.ServiceProvider.GetService<DbContext>();
var user = await dbContext.Users.FirstAsync(e => e.Name == name);
return user;
}
});
I have the same problem. is there a better solution?
I have also just hit this issue with a .NET Core implementation. Unfortunately, I think it's something that most people will hit if they use EF with the default settings, as the DbContext is scoped by default. This means that not only does this issue arise for data loaders, but also potentially query requests that contain multiple queries as they are executed in parallel.
Like DSilence suggested, you could create new a DbContext for each resolver, but depending on the size of the DbContext this could be expensive creating the model each time (creating relationships etc).
For the data loader situation, it would be nice if there was a toggle (off by default) which could have the data loaders dispatch sequentially instead of in parallel.
you can work around this by implementing your own execution strategy
https://github.com/graphql-dotnet/graphql-dotnet/pull/1026
public class EfDocumentExecuter : DocumentExecuter
{
protected override IExecutionStrategy SelectExecutionStrategy(ExecutionContext context)
{
if (context.Operation.OperationType == OperationType.Query)
{
return new SerialExecutionStrategy();
}
return base.SelectExecutionStrategy(context);
}
}
Hello, I have the same issue. Can someone give a complete example of the execution strategy class along with their startup.cs file please.
Thanks
I solved this by injecting Func<DbContext> instead of DbContext and instantiate a new DbContext every time I need one. This way DbContext is never shared across multiple threads.
Register Func<DbContext> in Startup.cs after MyDbContext is registered:
services.AddDbContext<MyDbContext>(options => options.UseSqlServer(Configuration.GetSection("ConnectionStrings")?["DefaultConnection"]), ServiceLifetime.Transient);
services.AddTransient<Func<MyDbContext>>(options => () => options.GetService<MyDbContext>());
Use Func<DbContext> in Queries. Example:
public class PostType : ObjectGraphType<Post>
{
public PostType(Func<MyDbContext> dataContext)
{
Field(t => t.Id);
Field(t => t.CreatedAt);
FieldAsync<NonNullGraphType<UserType>>(
"user",
resolve: async context =>
{
using (var dc = dataContext())
return await dc
.Users
.FirstAsync(u => u.Id == context.Source.UserId);
});
FieldAsync <NonNullGraphType<ListGraphType<CommentType>>>(
"comments",
resolve: async context =>
{
using (var dc = dataContext())
return await dc
.Comments
.Where(c => c.PostId == context.Source.Id)
.OrderBy(c => c.CreatedAt)
.ToListAsync();
});
}
}
I'm not sure if I configured this properly, so I'd love to get some feedback on this.
See #1310
You can't use adddbcontext.
It means "scoped" if you use a classic web api.
But if you use inside graphql middleware that you register as singleton It will be instantiate just at app startup!
Here a complete implementation of a solution with some extensions and tricks.
You have factory pattern for dbcontext example.
And have also an extension that created automatically dto models into objectgraphtype and inputobjectgraphtype
https://github.com/graphql-dotnet/graphql-dotnet/issues/576#issuecomment-626661695
@fenomeno83 Thanks for your feedback. I'm trying to understand your comment here:
You can't use adddbcontext.
It means "scoped" if you use a classic web api.
So this is the way I register my context using AddDbContext:
services.AddDbContext<MyDbContext>(options => options.UseSqlServer(Configuration.GetSection("ConnectionStrings")?["DefaultConnection"]), ServiceLifetime.Transient);
services.AddTransient<Func<MyDbContext>>(options => () => options.GetService<MyDbContext>());
The scope is set as transient and a new one is generated on every query (see my example above). Is that still a problem? Isn't a new context used for every query this way? Perhaps not the most efficient, but threads and race conditions shouldn't be a problem this way. Did I misunderstand?
@fenomeno83 Thanks for your feedback. I'm trying to understand your comment here:
You can't use adddbcontext.
It means "scoped" if you use a classic web api.So this is the way I register my context using
AddDbContext:services.AddDbContext<MyDbContext>(options => options.UseSqlServer(Configuration.GetSection("ConnectionStrings")?["DefaultConnection"]), ServiceLifetime.Transient); services.AddTransient<Func<MyDbContext>>(options => () => options.GetService<MyDbContext>());The scope is set as transient and a new one is generated on every query (see my example above). Is that still a problem? Isn't a new context used for every query this way? Perhaps not the most efficient, but threads and race conditions shouldn't be a problem this way. Did I misunderstand?
In my example everything is registered as singleton because graphql main container is singleton (everything like repositories, services, factory dbcontext, inside graphql singleton middleware no need to register as transient/scoped, because will be instantiate only at first request (1,2,n times, but only at first request; so use singleton because is useless instantiate n times at first request)
Use this code:
var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>();
optionsBuilder.UseSqlServer(Configuration.GetConnectionString("DefaultConnection"));
services.AddSingleton(s => new Func<MyDbContext>(() => new MyDbContext(optionsBuilder.Options)));
@fenomeno83
In my code, I'm injecting a new factory that always returns a new DbContext. In your example, it looks like you're injecting a singleton Factory that always returns a new DbContext. Aren't they essentially the same, as you end up with a new DbContext every time the factory is called?
Yes, maybe are the same.
You could use singleton in your factory.
@fenomeno83
I think returning a singleton DbContext from the factory would be problematic, as DbContext is not thread-safe. That's why I'm (and also you) are returning a new DbContext from the factory every time. Does that sound right?
Try to change only this
services.AddSingleton<Func<MyDbContext>>(options => () => options.GetService<MyDbContext>());
Is factory Singleton, not dbcontext that you instantiate explicitally
@fenomeno83 Thanks for the suggestion, but what is the benefit of making that change?
That at first request you don't instantiate many times the factory
Il ven 15 mag 2020, 09:18 Johnny Oshika notifications@github.com ha
scritto:
@fenomeno83 https://github.com/fenomeno83 Thanks for the suggestion,
but what is the benefit of making that change?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/graphql-dotnet/graphql-dotnet/issues/863#issuecomment-629073689,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAOMGUZWNA6A4LMSX3H2TYLRRTUD7ANCNFSM4F3ZT42Q
.
@fenomeno83 Thanks for the suggestion, but what is the benefit of making that change?
That at first request (and only at first) you don't instantiate many times the factory (many times how many is injected in your project)
@fenomeno83 Thank you for your suggestion. I changed my factory to a singleton.
Guys, please look at #1310 and this project. There you may find the answers.
Here a full implementation on .net core 3.1 with scoped problem solved
https://github.com/fenomeno83/graphql-dotnet-globalization-demo
you can work around this by implementing your own execution strategy
1026
public class EfDocumentExecuter : DocumentExecuter { protected override IExecutionStrategy SelectExecutionStrategy(ExecutionContext context) { if (context.Operation.OperationType == OperationType.Query) { return new SerialExecutionStrategy(); } return base.SelectExecutionStrategy(context); } }
This unfortunately turns dataloader off for me resulting in SELECT N+1.
Most helpful comment
you can work around this by implementing your own execution strategy
https://github.com/graphql-dotnet/graphql-dotnet/pull/1026