Graphql-dotnet: ParallelExecution in queries

[sergeyshaykhullin]

Problem

I am using EFCore + DataLoaders and trying to query

{
  root {
    nested1 {
      nomatter
    }
    nested2 {
      nomatter 
    }
  }
}

Each nested node using DataLoader with access to DbContext from UserContext from HttpContext scope. Executing both in parallel will fail with DbContext A second operation started on this context before a previous operation completed.
I found https://github.com/graphql-dotnet/graphql-dotnet/issues/889 and https://github.com/graphql-dotnet/graphql-dotnet/pull/891 about parallel execution in same nested nodes and don't get why it is. DbContext is not working with parallel access =(

Summary

I found 2 workarounds

• Use DbContextPool and get pooled DbContext only for query nodes
• Write custom DocumentExecuter

But they both is poor. Why i should write custom document executor or fall down in performance just to integrate EFCore? =/

Are there any better solutions?

Ideas

Maybe we need to move configuration of execution strategies to Options?

options.ExecutionStrategies.Query = new ...();
options.ExecutionStrategies.Mutation= new ...();
options.ExecutionStrategies.Subscription = new ...();

With default implementation to get from options or override SelectExecutionStrategy in derive Executer

[sungam3r]

This question has already been raised several times. The problem is that you use DbContext in a way that it is not intended to be used. You cannot use the same DbContext for different resolvers. Use pooling. Alse see #648 - it discusses general dependency injection issues.

[sungam3r]

Why i should write custom document executor

You don’t have to do this at all. The root of the problem is not this, but the choice of the wrong lifetime of the used DbContext.

[floyd-may]

I ran into the same issue. In my particular use case, having the DbContext lifetime set to Scoped is the right choice for broader reasons outside just GraphQL. Given that parallel execution is primarily a performance optimization, would it be better to consider parallel execution as something to opt-in to rather than opt-out? It seems that serial is the safer default.

[sungam3r]

Parallel execution of query fields is the part of GraphQL spec.

[floyd-may]

I agree that parallel execution is one of the benefits of GraphQL; however, I don't see anything in the spec that _requires_ parallel execution. It says _can_ and _may_ but certainly not _must_. It certainly doesn't harm compliance with the spec to execute in serial, and allow parallelism to be opt-in rather than opt-out. I understand that this would probably be a breaking change, so (assuming semver) it wouldn't be appropriate prior to 3.x (if it's possible to get it in). I do believe that changing the default from parallel to serial would save many people a headache.

[sungam3r]

I do believe that changing the default from parallel to serial would save many people a headache.

Rather, on the contrary, everyone will encounter the fact that their queries have begun to work prohibitively long. For what?

Just setup your own DocumentExecuter and override SelectExecutionStrategy. Default implementation is very simple.

I'm sure it’s better to leave the parallelization for queries on by default.

[ssougnez]

I'm having the same concern as @floyd-may. I'm using AddDbContextPool to improve performance of my application and therefore, lifetime of DbContext is "Scoped". However, it means that I'm unable to execute parallel queries with GraphQL as my DbContext is not transient (which is not an option).

@sungam3r > You said to use "pooling" but what do you have in mind ? Indeed, I am using pooling and it does not solve the issue in any way, I'm still getting the same issue :-)

[sungam3r]

it does not solve the issue in any way, I'm still getting the same issue

Because you use the same dbcontext instance.

Not everyone immediately understands this situation with the dbcontext pooling (and with pooling in general). Let me try to explain. You are probably used to thinking that if an object has "scoped" lifetime, then this implies:

1. As part of a scope (http request for ASP.NET Core), you always get the same instance.
2. Upon completion of the scope, this object is disposed and is no longer suitable for use.

So, in the case of using the pool, both of these statements are not true. The best way to understand what is happening and how it works is to look at the source code. Let's look at the second statement first. Here is a DbContext.Dispose method:
```c#
public virtual void Dispose()
{
if (this._dbContextPool != null || this._disposed) <---- !!!!!!!!!!!!!
return;

  IDbContextDependencies contextDependencies = this._dbContextDependencies;
  if (contextDependencies != null)
    contextDependencies.InfrastructureLogger.ContextDisposed(this);
  this._disposed = true;
  this._dbContextDependencies?.StateManager.Unsubscribe();
  this._serviceScope?.Dispose();
  this._dbContextDependencies = (IDbContextDependencies) null;
  this._changeTracker = (ChangeTracker) null;
  this._database = (DatabaseFacade) null;
}

From this it is clear that the context remains fully operational if received from the pool (connected to it).

Let's look at the first statement. Here is `AddDbContextPool` method:
```c#
   public static IServiceCollection AddDbContextPool<TContextService, TContextImplementation>(
      [NotNull] this IServiceCollection serviceCollection,
      [NotNull] Action<IServiceProvider, DbContextOptionsBuilder> optionsAction,
      int poolSize = 128)
      where TContextService : class
      where TContextImplementation : DbContext, TContextService
    {
      Check.NotNull<IServiceCollection>(serviceCollection, nameof (serviceCollection));
      Check.NotNull<Action<IServiceProvider, DbContextOptionsBuilder>>(optionsAction, nameof (optionsAction));
      if (poolSize <= 0)
        throw new ArgumentOutOfRangeException(nameof (poolSize), CoreStrings.InvalidPoolSize);
      EntityFrameworkServiceCollectionExtensions.CheckContextConstructors<TContextImplementation>();
      EntityFrameworkServiceCollectionExtensions.AddCoreServices<TContextImplementation>(serviceCollection, (Action<IServiceProvider, DbContextOptionsBuilder>) ((sp, ob) =>
      {
        optionsAction(sp, ob);
        CoreOptionsExtension extension = (ob.Options.FindExtension<CoreOptionsExtension>() ?? new CoreOptionsExtension()).WithMaxPoolSize(new int?(poolSize));
        ((IDbContextOptionsBuilderInfrastructure) ob).AddOrUpdateExtension<CoreOptionsExtension>(extension);
      }), ServiceLifetime.Singleton);
      serviceCollection.TryAddSingleton<DbContextPool<TContextImplementation>>((Func<IServiceProvider, DbContextPool<TContextImplementation>>) (sp => new DbContextPool<TContextImplementation>((DbContextOptions) sp.GetService<DbContextOptions<TContextImplementation>>())));
      serviceCollection.AddScoped<DbContextPool<TContextImplementation>.Lease>();

     // !!!!!!!!
     serviceCollection.AddScoped<TContextService>((Func<IServiceProvider, TContextService>) (sp => (TContextService) sp.GetService<DbContextPool<TContextImplementation>.Lease>().Context)); 
      return serviceCollection;
    }

And one more piece of the puzzle, DbContextPool.Lease:
```c#
public sealed class Lease : IDisposable
{
private DbContextPool _contextPool;

  public Lease([NotNull] DbContextPool<TContext> contextPool)
  {
    this._contextPool = contextPool;
    this.Context = this._contextPool.Rent();  <--- !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  }

  public TContext Context { get; private set; }

  void IDisposable.Dispose()
  {
    if (this._contextPool == null)
      return;
    if (!this._contextPool.Return(this.Context))
    {
      this.Context.SetPool((IDbContextPool) null);
      this.Context.Dispose();
    }
    this._contextPool = (DbContextPool<TContext>) null;
    this.Context = default (TContext);
  }
}

```

I will not show you the implementation of the Rent and other methods (there is already too much code), I think you yourself already understand that Rent will create a new dbcontext if it does not find any in the pool. To verify this, simply call serviceProvider.GetService<YourContext>() twice and compare results with object.ReferenceEquals(). You will get different instances.

The last question that you may have is how to get 2 different instances without resorting to explicitly calling serviceProvider.GetService<YourContext>(). For myself, I solved this problem with Func<T>. See #1160 . I hope this explanation helps.

[ssougnez]

I didn't have time to check that yet but I actually don't have the issue but I'm not sure why.
At first, I created a new class inheriting from DocumentExecuter and overridden the method to indicate that Query had to use the Sequential thingy.
At first it worked, but then, I configured my server a bit more by adding a data loader to avoid the N+1 issue. I struggled a bit because whatever I was doing, I was still seeing N+1 queries... Until I removed my custom document executer.
When I did so, the data loader started to work as expected.
Then out of curiosity, I tried to executed two queries at the same times and the parallel issue never occurred again.
Looks like using the data loader avoid the parallel issue. My suspicion is that as it "batch" the query, this one gets executed after the first one => sequentially.
However, it does not explain why if I query two separated fields, I still don't have the issue...

[sungam3r]

The problem could be anything. I would start by looking at which classes you are injecting the DbContext into and with which lifetime you are configuring them in the container.

[sungam3r]

@sergeyshaykhullin Is this issue still actual?

[sergeyshaykhullin]

@sungam3r, no. I solved it with DocumentExecuter.SelectExecutionStrategy override and it works fine, thanks!

[sungam3r]

@ssougnez Sorry, I was wrong about

both of these statements are not true.

Within one request, a reference to the same context is really returned. The pool helps reuse context between different requests, but within the same request there will always be the same context.

At one of our projects, we encountered a problem that required a return to the discussion of this issue. During the discussion, we came to the conclusion that the nature of GraphQL execution algorithms is such that it will conflict with the concept of well-known scopes (HTTP). We can talk about "scoped scope". There is little that can be done with this, except manually managing the lifetime of the some objects (DbContext in our case).

I solved it with DocumentExecuter.SelectExecutionStrategy override and it works fine, thanks!

@sergeyshaykhullin I must say that although this is a solution, it is achieved through a large degradation of performance. You sacrifice parallelism only because you cannot get the desired object from the DI.

If, nevertheless, I return to how original issue can be solved, then I see the answer as follows.

1 [harder]. We need to create our own scope (GraphQLResolverScope) and use DbContext in this scope. The difficulty lies in the fact that these scopes can be nested in each other, theoretically unlimited level deep. It follows that the existing methods such AddDbContextPool are not suitable. We need to invent something new. Relates to #1345, #1376 . There is also #1160 , but it is not directly related to the problem.
2 [easier]. Configure and use DbContext as transient.

However, it means that I'm unable to execute parallel queries with GraphQL as my DbContext is not transient (which is not an option).

Why not an option?

I didn't have time to check that yet but I actually don't have the issue but I'm not sure why.

Perhaps this is race conditions. We reproduce the error after about 10 attempts (requests).

[floyd-may]

There are very good reasons why it's best to prefer the lifetime of DbContext to be scoped rather than transient. Consider that many hosted databases in cloud services (AWS, Google Cloud, Azure) are very limited in the number of concurrent connections that they allow. By preferring transient DbContexts, in effect, the number of concurrent connections that a given GraphQL query opens is roughly proportional to the complexity of the query. This creates a situation where complex queries can inadvertently DoS the system by exhausting the available connections at the server. By using a scoped lifetime for the DbContext, it limits each HTTP request to a single database connection at a time.

Using DataLoader to prevent the N+1 problem makes a big difference, naturally, but I think you'll find that scoped lifetime for DbContext is often an immovable requirement for many systems, leaving parallelism a problematic default.

[sungam3r]

Consider that many hosted databases in cloud services (AWS, Google Cloud, Azure) are very limited in the number of concurrent connections that they allow.

OK. This problem in one form or another can be solved by means of DAL level connection pooling. SQL Server for example: Max Pool Size. It seems to me to solve this problem at the GraphQL / DI level incorrectly (big hammer).

By using a scoped lifetime for the DbContext, it limits each HTTP request to a single database connection at a time.

This is clear. And this is too demanding.

[Shane32]

There are very good reasons why it's best to prefer the lifetime of DbContext to be scoped rather than transient.

Agreed. My application has many "business logic layer" classes that utilize DI to obtain references to database objects. For instance, the ShoppingCart class might return a number of Products, while the ProductThumbnail class might take a Product and add a thumbnail if none exists. If the DbContext was transient (and therefor different in the Thumbnail class), it would not recognize any changes made to the Product object unless it was reattached to the DbContext. So basically, if the DbContext was a Transient scope, none of my "business logic layer" classes would interoperate with each other properly. This is just my scenario, of course.

As for pooling, whether the application is using DbContext pooling (or indeed if SQL Server Connection Pooling is in use) really has no bearing on the hard fact that a DbContext can only be in use by a single thread at any point in time. And therefor one of the following MUST be true if DI is used to create the DbContext:

• The DbContext must be registered as Transient
• The DI scope must be different than the other thread
• The resolvers must run in series, rather than parallel

Perhaps some people can use a transient DbContext. The suggestion in #1345 allows for a different scope for each concurrently run resolver when executing. But at present, most people resort to running the resolvers in series (SerialExecutionStrategy).

Note that this ignores the actual method of defining the objects that are need to be obtained via DI. This is covered in #1345 and #1160 in different ways.

[floyd-may]

Agreed. My application has many "business logic layer" classes that utilize DI to obtain references to database objects. For instance, the ShoppingCart class might return a number of Products, while the ProductThumbnail class might take a Product and add a thumbnail if none exists. If the DbContext was transient (and therefor different in the Thumbnail class), it would not recognize any changes made to the Product object unless it was reattached to the DbContext. So basically, if the DbContext was a Transient scope, none of my "business logic layer" classes would interoperate with each other properly. This is just my scenario, of course.

This is a classic case of the DbContext serving as the Unit of Work pattern (https://www.martinfowler.com/eaaCatalog/unitOfWork.html) and is incredibly common. Although it may be convenient to hand-wave scoped DbContext lifetimes away, this is a very real issue for many real-world applications. By defining the lifetime as transient, you destroy the ability of the DbContext to serve as a unit of work.

[sungam3r]

But at present, most people resort 

What is the basis of this statement?

[sungam3r]

By defining the lifetime as transient, you destroy the ability of the DbContext to serve as a unit of work.

Disagree. Only the degree of manual control changes. DbContext already by itself is Unit of Work. You tighten requirements by considering its application only through DI (as it seemed to me).

However, I do not agitate to use DbContext as transient. In fact, in practice I come across all the described scenarios including transient.

[Shane32]

Just from the basis of all the posted issues and recommended solutions pertaining to using EF with this library. Also the GraphQL.EntityFramework library uses a serial execution strategy. However, perhaps your experience would indicate otherwise. My comment of course related specifically to EF and wouldn’t necessarily hold true for other uses of graphql.

[sungam3r]

Here are some more considerations. Let's abstract from specifically DbContext. Consider an arbitrary class used by resolvers. What are the requirements for it? If an object of this class is shared by several resolvers, then it is obvious that it should be designed taking into account the parallel calls to the resolvers. If the class is designed for single-threaded use (as DbContext does), then we cannot jump above our heads.

There are 4 options:
1) do not use this class, use some other, concurrent friendly
2) do not share instance of this class, create and use different instances, in our case, this somehow means using transient
3) adjust all the environment to the requirements of the class design, in our case, this means using SerialExecutionStrategy
4) adjust the environment to the requirements of the class design only in the minimum necessary set of places, that is, where it is accessed directly. I'm talking about synchronization (lock).

In other words, I want to say that the problem is not in GraphQL, but our desire to sit on two chairs at the same time. You can walk around the problem for as long as you want, trying to find the very ideal solution, but it simply does not exist. Ultimately you are left with a choice of one of the four options above.

[Shane32]

Or it could create a different scope for each resolver. (Falls into either box 2 or 3 above, but doesn’t use transient and still runs in parallel) PR #1345 can do this.

And the PR #1376 draft can define only certain resolvers to run in series, or only certain resolvers to create a different scope. (Falls into box 4 above). Plus the PR allows for dataloaders in serial mode.

I’ve been thinking about synchronization/lock also. I’m not sure exactly what you were thinking, but if it locked on the dbcontext it seems that it would force all EF queries to run in series. Any resolver without await will run in series anyway. So if the project was mostly an EF wrapper, it would run almost the same as a serial execution strategy. Did you have another idea?

[sungam3r]

it would force all EF queries to run in series

It would force all EF queries on the same DbContext instance to run in series. I mean one lock object by one DbContext.

Or it could create a different scope for each resolver.

This is option 2 + side effects for all other types in the container. Since scope is narrowed from the entire request to a single resolver, scoped objects will be created more often. In general, the architecture of nested scopes requires quite a bit of discussion. Not very obvious effects appear.

[floyd-may]

This is option 2 + side effects for all other types in the container. Since scope is narrowed from the entire request to a single resolver, scoped objects will be created more often. In general, the architecture of nested scopes requires quite a bit of discussion. Not very obvious effects appear.

IIUC the microsoft dependency injection stuff isn't intending to support nested scopes:
https://github.com/dotnet/aspnetcore/issues/2351#issuecomment-430764560

If the desired solution is nested scopes, then it requires something other than the Microsoft DI APIs. Since ASP.Net Core and the Microsoft DI APIs are pretty coupled to one another, a solution that is idiomatic with regards to those two things would probably be preferable.

[Shane32]

I mean one lock object by one DbContext.

Ah, okay. The majority of my projects utilize a single DbContext.

nested scopes

I was not aware that some DI providers provide nested scopes. I assume MS's scope provider is a singleton, and therefor each scope is "parallel" to other scopes - ? I would assume that even with autofac, they would all be direct children scopes of the http scope, which doesn't seem to be a problem. I will need to read up on it some more for sure.

[sungam3r]

After this comment, I am no longer sure that they are really nested.

[johnnyoshika]

This is a great discussion!

Although there's a flaw in my use of transient DbContext, and even worse as I new up a new DbContext every time I need it, I'm going to post my solution in case it helps anyone. I'm also open to feedback.

Note that I posted this same comment here: https://github.com/graphql-dotnet/graphql-dotnet/issues/863#issuecomment-578616371

---

I'm 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();
                });
        }
    }

[togakangaroo]

I think I might agree with @floyd-may that while this isn't technically GraphQL's problem, the fact that it is such a common use case and a common issue might make sense for GraphQL to handle things in a single thread by default and have a well documented and caveated option to enable handling things in parallel.

The UnitOfWork issue is not a super minor one too. It is very reasonable when using a single data store to want all queries and mutations to occur on the same unit of work and only commit at the end of the http request.

Furthermore, I'm not sure that handling things in parallel on different threads is actually that much faster. For one thing, my gut-guess is that the big win would be to minimize interactions with the database far more than to parallelize them. For another, that makes me nervous about what it might do to the really fast request handling numbers we've seen with dotnet core benchmarks that come largely (I think) from more efficient use of the TPL.

It's your project, I'm not here making PRs, but, just another point of view to consider.

[sungam3r]

might make sense for GraphQL to handle things in a single thread by default and have a well documented and caveated option to enable handling things in parallel.

You can make execution serial or any you want by overriding protected virtual IExecutionStrategy SelectExecutionStrategy(ExecutionContext context). I have already said that the execution of all requests sequentially by default will lead to unjustified delays. I also said that GraphQL is not tied to working with databases. You look at the problem based on your narrow task and try to generalize it to all scenarios. What would happen if the DbContext was originally designed with multi-threaded scenarios in mind? And what if support for this is added in a future version of EF? Will we need to change our default behavior one more time?

Or imagine that now in this thread there will be a person who uses GraphQL as an API gateway, which sends requests to various independent services under the hood (and this is really one of the main work scenarios). How should he take your suggestions for sequential processing? Does he have the right to require by default to execute all requests in parallel? This is a conflict of interest due to the specific application domain where GraphQL is used.

That is why the opportunity is given to redefine the execution strategy. This requires writing just a few lines of code. In this case, the default behavior does not impose assumptions/restrictions on the execution and executes the resolvers in parallel.

In summary, based on the above explanations, I would like to clarify the question of whether the default behavior will be changed. No, it will not. Nevertheless, the question of how to make working with EF more convenient and safe is open.

[mikelneo]

We can use custom ExecutionStrategy for EF (and other EF-like scenarios).
For example create ExecutionStrategyDirective schema directive and apply it on fields that works with EF context. Like:
```c#
///

/// GraphQL type for .
///

public class MyClassType : ObjectGraphType
{
public MyClassType()
{
// for EF resolve fields
Field(x => x.Field1, nullable: false).Directive(new ExecutionStrategyDirective(Strategy.Serial));
Field(x => x.Field2, nullable: false).Directive(new ExecutionStrategyDirective(Strategy.Serial));

    // for EF not related fields
    Field(x => x.Field3, nullable: false).Directive(new ExecutionStrategyDirective(Strategy.Parallel));
    ...

`` Then inCustomExecutionStrategyuse this knowlage to resolve some fields not parallely. Probably thisExecutionStrategyDirectivecan be part ofgraphql-dotnet`.

[sungam3r]

To clarify previous comment - @mikelneo talks about #1451.

[Shane32]

@mikelneo PR #1376 is a bit off topic, but it does contain this type execution strategy, where you can tell certain fields to run in serial, and others in parallel. Note that in an EF scenario with a single datacontext, it's often a bit pointless, as the resolvers that hit EF are set to run in serial, and the others don't contain an await and so run serially even if in a parallel execution strategy.

[fenomeno83]

Here a full working implementation using factory pattern to use dbcontext that solves parallel execution

I've implemented also an extension that autogenerates objectgraphtype and inputobjectgraphtype starting from dto model

https://github.com/graphql-dotnet/graphql-dotnet/issues/576#issuecomment-626661695

[fenomeno83]

Here a full implementation on .net core 3.1 with scoped problem solved
https://github.com/fenomeno83/graphql-dotnet-globalization-demo

[sungam3r]

This topic was covered in docs for 3.0.0 release.

[fenomeno83]

This topic was covered in docs for 3.0.0 release.

Hi.
When I try to open documentation, I have not found page

[sungam3r]

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/docs2/site/docs/guides/known-issues.md#entity-framework-concurrency-issues

[fenomeno83]

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/docs2/site/docs/guides/known-issues.md#entity-framework-concurrency-issues

"Finally, you can create a scope within each field resolver that relies on Entity Framework or your other scoped services. Please see the section on this in the dependency injection documentation."

If I click on link of dependency injection documentation I've 404

[sungam3r]

Should be fixed in e0d044ce6d84c91088b6e92a956f387ef1c4881e