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

Capture events in constructor configuration when using context pooling #19193

Closed
KenBrannigan opened this issue Dec 5, 2019 · 4 comments · Fixed by #24768
Closed

Capture events in constructor configuration when using context pooling #19193

KenBrannigan opened this issue Dec 5, 2019 · 4 comments · Fixed by #24768
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@KenBrannigan
Copy link

I have a ASP.NET Core 3.0 application using .NET Core 3.0 and EF Core 3.0. I am using AddDbContextPool to register my DbContext:

services.AddDbContextPool<IMyDbContext, MyDbContext>(
            optionsAction: options => options.UseSqlServer(
                connectionString: Configuration.GetConnectionString("MyDatabase"))
);

In my DbContext constructor, I attach to two events of the ChangeTracker:

public MyDbContext(DbContextOptions<MyDbContext> options)
            : base(options)
{
            // Attach to our important events
            ChangeTracker.StateChanged += ChangeTracker_StateChanged;
            ChangeTracker.Tracked += ChangeTracker_Tracked;
}

The first time I run the application the events are fired when I add items to my context. Once the page is done loading I make another request but none of my events fire this time when adding items to my context. It seems when it pulls a context out of the pool it no longer has the event handlers registered.

I have no issues when I use regular AddDbContext. Is this expected behavior or is there a different way I should register my event handlers?

@AndriySvyryd
Copy link
Member

@KenBrannigan This is by design. Each time you return a MyDbContext instance from the pool it gets all its state removed, so these events should be added whenever the context is requested from the pool. #17086 would make this simpler

@ajcvickers
Copy link
Member

@AndriySvyryd for other things that can be set in the constructor we make an effort to reset the the values that were set after the constructor finished. We should discuss whether or not to try to do this here.

@ajcvickers ajcvickers reopened this Dec 6, 2019
@ajcvickers ajcvickers changed the title ASP.NET Core AddDbContextPool not firing ChangeTracker events Stop trying to capture constructor configuration when using context pooling Dec 7, 2019
@ajcvickers
Copy link
Member

We discussed this in triage and decided that rather than continue along the path we have been on with capturing and resetting constructor configuration, we should implement #17086 and direct people to use that for per-request configuration of the instance.

We may then choose to stop capturing configuration at all. We could even "Reset" the context after it has been constructed.

@ajcvickers ajcvickers added this to the Backlog milestone Dec 7, 2019
@ajcvickers ajcvickers self-assigned this Dec 7, 2019
@ajcvickers ajcvickers modified the milestones: Backlog, 6.0.0 Nov 2, 2020
ajcvickers added a commit that referenced this issue Apr 26, 2021
…e pool.

Fixes #19193
Fixes #17086

Notes:
* We now no longer reset context events, as per #19193
* However, I left the code that resets options like QueryTrackingBehavior since that still seems useful, and makes this less of a break
* Added events to DbContext. These are easy and decoupled, but sync only.
* A derived DbContext can override the On... event methods. This allows async hooks.
* DbContextFactory now has a CreateDbContextAsync method such that a context can be rented from the pool and the hook applied in an async context.
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed needs-design labels Apr 26, 2021
@ajcvickers ajcvickers modified the milestone: 6.0.0 Apr 26, 2021
@ajcvickers ajcvickers removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 6, 2021
@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed breaking-change labels Jun 9, 2021
@AndriySvyryd
Copy link
Member

We discussed it in triage again and decided to go back to resetting the state to what was captured after the constructor.

@AndriySvyryd AndriySvyryd changed the title Stop trying to capture constructor configuration when using context pooling Capture events in constructor configuration when using context pooling Jun 9, 2021
AndriySvyryd pushed a commit that referenced this issue Jun 9, 2021
…e pool.

Fixes #19193
Fixes #17086

Notes:
* We now no longer reset context events, as per #19193
* However, I left the code that resets options like QueryTrackingBehavior since that still seems useful, and makes this less of a break
* Added events to DbContext. These are easy and decoupled, but sync only.
* A derived DbContext can override the On... event methods. This allows async hooks.
* DbContextFactory now has a CreateDbContextAsync method such that a context can be rented from the pool and the hook applied in an async context.
AndriySvyryd pushed a commit that referenced this issue Jun 11, 2021
…e pool.

Fixes #19193
Fixes #17086

Notes:
* We now no longer reset context events, as per #19193
* However, I left the code that resets options like QueryTrackingBehavior since that still seems useful, and makes this less of a break
* Added events to DbContext. These are easy and decoupled, but sync only.
* A derived DbContext can override the On... event methods. This allows async hooks.
* DbContextFactory now has a CreateDbContextAsync method such that a context can be rented from the pool and the hook applied in an async context.
AndriySvyryd pushed a commit that referenced this issue Jul 28, 2021
…e pool.

Fixes #19193

Notes:
* We now no longer reset context events, as per #19193
* However, I left the code that resets options like QueryTrackingBehavior since that still seems useful, and makes this less of a break
* Added events to DbContext. These are easy and decoupled, but sync only.
* A derived DbContext can override the On... event methods. This allows async hooks.
* DbContextFactory now has a CreateDbContextAsync method such that a context can be rented from the pool and the hook applied in an async context.
AndriySvyryd pushed a commit that referenced this issue Jul 29, 2021
…e pool.

Fixes #19193

Notes:
* We now no longer reset context events, as per #19193
* However, I left the code that resets options like QueryTrackingBehavior since that still seems useful, and makes this less of a break
* Added events to DbContext. These are easy and decoupled, but sync only.
* A derived DbContext can override the On... event methods. This allows async hooks.
* DbContextFactory now has a CreateDbContextAsync method such that a context can be rented from the pool and the hook applied in an async context.
AndriySvyryd added a commit that referenced this issue Jul 29, 2021
Fixes #19193

Co-authored-by: Andriy Svyryd <Andriy.Svyryd@microsoft.com>
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-rc1 Aug 12, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc1, 6.0.0 Nov 8, 2021
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants