Aspnetcore: Run every healthcheck in its own scope

Created on 26 Sep 2019  路  12Comments  路  Source: dotnet/aspnetcore

Is your feature request related to a problem? Please describe.

Right now adding multiple different checks using the same DbContext type will result in some of them being failed, because the DbContext might already be disposed.

Default Lifetime of DbContext is scoped, it would be a shame to have to change that to transient for HealthChecks to work properly in this scenarop.

Describe the solution you'd like

Please change or add options to configure this behavior to run every healthcheck in its own scope,

Additional context

working repro here:
https://github.com/ThomasBergholdWieser/healthchecksrepro

Just execute the /health endpoint a few times, it should happen pretty often. Change the Lifetime to Transient to make it work.

affected-medium area-healthchecks bug severity-major

Most helpful comment

Looks like the issue really mostly is that the checks run in parallel. This makes using DbContext checks hard since DbContext is not thread safe.

This is a bit interesting since:

  • Most other user code is executed serially and thus doesn't create issues with scoped dependencies. I might be mistaken for that, but do other things like authorisation policies execute in parallel? This really is a bit of a surprise. Could lead other users into similar issues with other non-thread-safe libraries.
  • It will be hard to isolate and thread-safe custom checks or create a collection of own checks in addition to libraries that do have dependencies on scoped dependencies.

There would be two ways to mitigate this:

  1. Option to execute checks serially.
  2. Option to create scopes for each check.

1 should be a good start to help fix such issues. 2. would also fix issues where we can't control 3rd party checks. Like with EF where there wold be a race if executed serially but could have issues as well when one of the checks creates an instance in the outer scope and then the inner scopes resolve this object again (e.g. EF check that doesn't create its own scope).

All 12 comments

@ThomasBergholdWieser Thanks for contacting us.

Have you tried resolving an IServiceScopeFactory within your HealthCheck and using that to create your own scope and resolve the DbContext from there?

But that doesn't help that the inbox EF health check already created a dbcontext in the parent scope and the new sub-scope resolves the already created context from the parent scope?

Injecting the IServiceScopeFactory helps with this check, but somehow i am now getting weird issues with the default AddDbContext Check. I have updated my repro to include another version of the Custom Check with the service scope factory.

Here is an example response:

{
  "status": "Unhealthy",
  "statusComponents": [
    {
      "key": "AddDbContextCheck",
      "value": "Unhealthy",
      "desc": "An attempt was made to use the context while it is being configured. A DbContext instance cannot be used inside OnConfiguring since it is still being configured at this point. This can happen if a second operation is started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.",
      "ex": "An attempt was made to use the context while it is being configured. A DbContext instance cannot be used inside OnConfiguring since it is still being configured at this point. This can happen if a second operation is started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe."
    },
    {
      "key": "CustomCheckInjectedScopeFactory",
      "value": "Healthy",
      "desc": null,
      "ex": null
    },
    {
      "key": "CustomCheckInjectedContext",
      "value": "Healthy",
      "desc": null,
      "ex": null
    }
  ]
}

@ThomasBergholdWieser This goes beyond my knowledge. I'm looping in some folks to help here.

@rynowak @bricelam any ideas on how to integrate EF + Healthchecks??

Can you share the call-stack of the exception you're hitting?

health checks already creates a scope when it runs so all of your checks will share the same scope. https://github.com/aspnet/Extensions/blob/master/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs#L53

I expect the problems that you'd hit are related to executing checks concurrently which DbContext doesn't support. We should probably have an option to execute checks serially instead of in parallel.

Can I ask why you end up needing multiple health checks for the same DbContext? Is this because you want to run multiple queries on the same database and report their results separately as metrics?

We wanted to create both low-level technical health checks (AddDbContextCheck) and business health checks - e.g. "a lot of imports are failing", "work in progress is stacking up".

We assumed that health checks would be a good abstraction for composing health status. But apparently the lack of scoping with some EF specifics defies that.

I'd argue that in order to be a good compositional pattern, it should allow for isolation of each component or else it would force users to create an additional layer of abstraction (scope creation + registration of checks that need scope) for it to be usable without fearing unintended side effects of using some components together.

This may be a problem once libraries that add to / extend EF contexts (asp.net core identity?, hangfire, openiddict, ...) offer health check components that would otherwise not be able to run alongside one another.

But apparently the lack of scoping with some EF specifics defies that.

Sorry, I still don't get it - we create a scope for all health checks and run them in there. Can you explain how this is causing your problem?

I'll try your repro and get back to you.

Looks like the issue really mostly is that the checks run in parallel. This makes using DbContext checks hard since DbContext is not thread safe.

This is a bit interesting since:

  • Most other user code is executed serially and thus doesn't create issues with scoped dependencies. I might be mistaken for that, but do other things like authorisation policies execute in parallel? This really is a bit of a surprise. Could lead other users into similar issues with other non-thread-safe libraries.
  • It will be hard to isolate and thread-safe custom checks or create a collection of own checks in addition to libraries that do have dependencies on scoped dependencies.

There would be two ways to mitigate this:

  1. Option to execute checks serially.
  2. Option to create scopes for each check.

1 should be a good start to help fix such issues. 2. would also fix issues where we can't control 3rd party checks. Like with EF where there wold be a race if executed serially but could have issues as well when one of the checks creates an instance in the outer scope and then the inner scopes resolve this object again (e.g. EF check that doesn't create its own scope).

There would be two ways to mitigate this:

  1. Option to execute checks serially.
  2. Option to create scopes for each check.

By default, a service scope should be created for each check by HealthCheckService.

Checking serially is effective but it is more like workaround because checks should be able to run concurrently already just by having their own scope. Using only single scope for all checks like it is right now is also not safe, as the state of the services inside the scope might be altered by prior checks and it might affect the result of latter checks.

I'm also getting the 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. For more information on how to avoid threading issues with DbContext, see https://go.microsoft.com/fwlink/?linkid=2097913. error.

We have HealthChecks that take the DbContext as a constructor parameter.

How do we solve this?

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

cc @ajcvickers

Was this page helpful?
0 / 5 - 0 ratings

Related issues

KerolosMalak picture KerolosMalak  路  269Comments

clement911 picture clement911  路  129Comments

davidfowl picture davidfowl  路  126Comments

zorthgo picture zorthgo  路  136Comments

MaximRouiller picture MaximRouiller  路  338Comments