-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Is ManyServiceProvidersCreatedWarning
expected when adding services in OnConfiguring and "externally"
#30191
Comments
Yes. However, regardless of how they are added if the configuration or extensions results in changing singleton configuration, then it will ultimately result in the problem you are seeing. It's hard to say which configuration is resulting in this without a repro. |
Ok that helps. Like I said there's a bunch of moving parts so a simple repro is a little difficult. Had you said "no" then case closed and I'd have saved myself some considerable effort. Given that it should be valid, I will try and put together the most minimal thing I can |
So I was able to condense this down quite a bit. However it's still too big to fit in an issue, so I've created a repo for it: https://github.com/pinkfloydx33/ef30191 These are the lines/areas of interest: There are four different variants in the test-setup. The first one will throw the Many Service Providers error. The repro is configured for postgres; the connection string needs to be adapted. Now that I've stripped this down, it seems like it could actually be efcore.pg specific -- FYI @roji . I don't have a way to test with Sql Server though, sorry. |
I believe the issue here is |
In a normal application the guidance is to use only one datasource. We can't quite do that in our tests since we are hitting different containers (ie. servers), so I think we're going to need to eventually suppress the warning as we add more test classes. That should be alright for tests and is something suggested by (I think) you in one of the various issues I found on the topic. I guess my question becomes why does it only happen if you add the extensions from My main concern with the OP was that this wasn't going to rear it's head during normal application execution. Since we are using the pg features there correctly I don't think we'd ever encounter this. But given that the SP logic seems somehow dependent on where/how other extensions get added I'm not 100% convinced. |
@pinkfloydx33 Yes, its seems like it should generate the warning for both cases where |
Ping @roji |
Sorry for taking so long to look at this. So first, yeah, normal applications use a single data source (or a very small number thereof). If you're looking to use separate databases in testing so as to achieve isolation, you indeed need multiple data sources; and as each data source requires a separate service provider, that means that you'll have to disable the "Many Service Providers" in your tests. All that makes sense. Now, I looked into it, and I think there's a concurrency issue going on. The "Many Service Providers" error is reported inside the value factory lambda that we pass to the ConcurrentDictionary GetOrAdd (code). While ConcurrentDictionary guarantees thread-safety, and that only one key/value will end up in the dictionary, it doesn't guarantee that the value factories don't run concurrently - for the same key or for different ones. In other words, since the test classes are running in parallel, it's possible that due to a race condition, all will reach the check when none have completed the factory, and so _configuration still has zero elements. That means we can end up with many service providers - beyond the maximum of 20 - without triggering the error. I'm not exactly sure why the extension plays a role here, but as a quick test I dropped the error threshold from 20 to 10, and saw some tests start to fail; this means that if you add more test classes, you may also start to see failures. I'm not diving deeper into it, since it really does seem like a race condition. We can try to make the check tighter by first doing a TryGetValue (fast path), and only if that fails, take a lock on the dictionary and do the insertion; or we can see it as a "best effort" kind of warning (after all, 20 is a pretty arbitrary number). |
We are experiencing the same. As @roji said, each new database will generate a new Is there a way to disable the warning globally without going through each and every |
Today I fell into this pit, too. I am heavily paralleling test runs with xUnit and create a totally separate database and DbContexts for each test (npgsql, of course). So, yes, it is totally expected to have more than 20 service providers. workaround is: public static DbContextOptionsBuilder<TDbContext> UseMyDb<TDbContext>(
this DbContextOptionsBuilder<TDbContext> dbContextBuilder,
string connectionString,
bool inTests = false) where TDbContext : DbContext
{
var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString);
dataSourceBuilder.UseNodaTime();
var dataSource = dataSourceBuilder.Build();
dbContextBuilder
.UseNpgsql(
dataSource,
opt =>
{
opt.CommandTimeout(commandTimeout: 30);
opt.MigrationsHistoryTable("_ef_migrations_history", MyDbContext.SchemaName);
opt.UseNodaTime();
})
.UseSnakeCaseNamingConvention();
if (inTests)
{
// Suppress the ManyServiceProvidersCreated warning:
// parallel test runs are expected to create a lot of service providers
dbContextBuilder.ConfigureWarnings(builder => builder.Ignore(CoreEventId.ManyServiceProvidersCreatedWarning));
}
return dbContextBuilder;
} I am unsure whether EF Core made a good decision in opting for guiding the noob while bothering the experienced developer. |
Note that the plan is to change EFCore.PG to make NpgsqlDataSource scoped rather than singleton (for other reasons); this would also stop requiring a new EF service provider for different data source instances, which should make this all go away (tracked by npgsql/efcore.pg#3086). |
This is not a bug report but rather a request for guidance
Ask a question
Will adding extensions/configuration inside of
OnConfiguring
lead to Service Provider caching issues if you also add extensions via other other means (DI registration, test setup) external to the context?I am trying to figure out what is causing the
ManyServiceProvidersCreatedWarning
error to be emitted during our tests runs. I've read the various issues throughout the repository and I understand that it's usually indicative of SP caching not functioning due to some singleton service, etc. I've been trying to trace down the root cause and I've found a work-around that has me worried that we are configuring our contexts/applications incorrectly. I'm looking for some guidance.I don't have a minimal repro yet as there's a lot of moving parts. In the meantime I hope that something in my description below jumps out.
Include your code
We have a base
DbContext
class that we share throughout all of our projects. As part ofOnConfiguring
it adds a couple of extensions--naming conventions and an internal extension (see below)--guarded behindOptions.IsFrozen
In a normal application we register our contexts in DI via
AddDbContext
:We expect all of our applications to use the naming conventions and our custom extension. The
UseOurExtensionAndConfiguration
adds anIDbContextOptionsExtension
which in turn registers some interceptors and aConventionSetPlugin
and callsConfigureWarnings
to set the log level for a few relational events. We don't want to repeat it in every application which is why we've put it in our base context'sOnConfiguring
as highlighted above. So far this seemed to be working fine.Recently, however we began to experience the
ManyServiceProvidersCreatedWarning
in our tests. We use an xUnit class fixture that initializes Postgres containers (via Testcontainers), configures test-specificDbContextOptions
and provides a method for our test classes to create a new context. Each class gets it's own instance of the fixture (can't be shared due to some unrelated functionality I've elided) that looks like the following:The workaround?
We've started to notice that as we add more test classes we are hitting the
ManyServiceProvidersCreatedWarning
issue. After a lot of guess-and-check debugging, I discovered that adding the following two lines into theCreateOptions
method prevents this problem:You'll note that these are the same two methods that we are calling in
OnConfiguring
guarded by theIsFrozen
check. If either is omitted I get the errors again. It seems that EF builds the options twice--before the constructor and then again forOnConfiguring
--considers them different, and thus must create a new internal provider.Summary/Question
Is adding extensions/performing configuring guarded behind
IsFrozen
valid, particularly if we are going to provide additional configuration "outside" the context? Is there something downright wrong about what we're doing? What's the recommended pattern for stuff like this--do I need to consolidate all extension setup into one place?We're working on a set of greenfield applications that aren't getting much load at the moment. I'm worried that since we don't call these methods in our normal DI registration flow that we are going to eventually hit the same problem in our application as we do in our tests. If this is just likely due to the way our tests are set up I'd have no issue silencing the warning there,but I'd like to take preventative measures if this is likely a larger issue.
Include provider and version information
EF Core version: 7.0.2
Database provider: efcore.pg
Target framework: NET7.0
Operating system: Windows11, debian (containers)
IDE: VS 2022
The text was updated successfully, but these errors were encountered: