Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBatchScheduler default service change in HC14 (maybe intentional though?) #7810

Closed
jeffrimko opened this issue Dec 6, 2024 · 5 comments
Closed

Comments

@jeffrimko
Copy link

jeffrimko commented Dec 6, 2024

Product

Hot Chocolate

Version

14.0.0

Link to minimal reproduction

n/a

Steps to reproduce

This might not be a bug but wanted to check.

In HC13, the default IBatchScheduler was registered to HotChocolate.Fetching.BatchScheduler but it has changed to GreenDonut.AutoBatchScheduler starting in HC14. The behavior between these are different enough to cause issues if unaware, didn't see anything mentioned in the release notes but may have missed it. Heads up to anyone running into such issues, use IBatchHandler (which implements IBatchScheduler) instead since it is still registered to HotChocolate.Fetching.BatchScheduler in HC14.

If this change wasn't intentional, maybe the following line in InternalServiceCollectionExtensions.cs -> TryAddDefaultBatchDispatcher() should be changed?

// current line
services.TryAddScoped<IBatchScheduler, AutoBatchScheduler>();  

// maybe change to this?
services.TryAddScoped<IBatchScheduler>(sp => sp.GetRequiredService<IBatchHandler>());  

If this change was intentional, is there info somewhere explaining the change?

What is expected?

n/a

What is actually happening?

n/a

Relevant log output

Additional context

No response

@michaelstaib
Copy link
Member

This is not a bug and also not what is used when using HotChocolate. We decoupled the DataLoader from HotChocolate itself so it works also within a REST context. The AutoBatchScheduler is the default.

When a GraphQL request however starts we are initializing a new DataLoaderScope within which the execution engine takes over dispatching.

RequestExecutor.cs

if (scopeDataLoader)
{
    // we ensure that at the begin of each execution there is a fresh batching scope.
    services.InitializeDataLoaderScope();
}

The initializer will then create a new DataLoader scope for the current execution that is initialized with the current batching handler for the current execution engine.

internal static class DataLoaderServiceProviderExtensions
{
    public static void InitializeDataLoaderScope(this IServiceProvider services)
    {
        var batchHandler = services.GetRequiredService<IBatchHandler>();
        var dataLoaderScopeHolder = services.GetRequiredService<IDataLoaderScopeFactory>();
        dataLoaderScopeHolder.BeginScope(batchHandler);
    }
}

There is more work in the pipeline on the scoping part which also moves the task tracking into a custom TaskScheduler:
#7308

@smbecker
Copy link
Contributor

This isn't specifically about DataLoader. We were using IBatchScheduler outside of the context of DataLoader. This was still a change in behavior that was undocumented.

@michaelstaib
Copy link
Member

IBatchScheduler is an interface for DataLoader that is specifically meant to enable DataLoader. The observable execution logic for DataLoader is the same. The HotChocolate specific integration code was never meant to integrate with for external components.

That is why we did not mention it in the migration guide. We will do further changes in this area, so be aware. IBatchHandler and the manual batching will vanish completely with 15 once the new task scheduler comes. We have some major work still left on this one.

@michaelstaib
Copy link
Member

What is your use-case btw?

@smbecker
Copy link
Contributor

What is your use-case btw?

We are batching queries but not using DataLoader. Rather, we're using DbBatch where multiple graphql queries to different models would result in a single request to the database. We chose not to use DataLoader because it was too limiting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants