-
Notifications
You must be signed in to change notification settings - Fork 126
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
[FEATURE] Expose currently backgrounded factory tasks on DI scope #368
Comments
If my question or proposal are not welcome, I can close the question and try to solve my issue by wrapping around var tsc = new TaskCompletionSource();
addToWaitGroup(tsc)
try { factory(...) }
finally { tsc.SetResult() } |
Hi @Fube
Totally not the case, any and all ideas are more than welcome! Sorry for the delay, but honestly it has just been a rough time lately with not that much time left: after the mega crunch I did to get out with v2, various daily work stuff including a quite long commute, preparing for the MVP Summit, some help I'm trying to give to another project and... life in general. I'll try to dedicate some time to this in the next few days, will get back to you. |
Hi @jodydonetti! Thank you very much for your reply and your work on the library. |
Hi @Fube
Yes, this has come up multiple times in the past, mostly because of EF DbContext in ASP.NET where, typically, that is scoped to the http request being processed. Is this your case? Can you give me more context on what is your specific scenario, so I can maybe come up with something? It would be helpful. Anyway, in the case of EF the solution was to simply switch from an automatically scoped context to an explicit one, via a Here are some previous examples:
You can see that it worked well for them, so maybe you can start from there too? Let me kwnow.
Maybe, but I honestly feel like this is tackling the situation from the wrong perspective, because it then becomes a problem of events coordination, and passing contexts or similar around and... I don't know, it doesn't feel like it.
Just FYI, I tried exploring a "deferred scope dispose" as a potential way, even directly with the EF team (see here), but you can see from their answers that it's probably not the right call. In general I'd say that the simplest and more reasonable solution is to switch from an automatic scope disposal to a manual one, because in the end that is what you are trying to do, imho. Also, doing
Events are not the right place anyway, since they are very thing, do not pass too much data around and the execution is generally not sync.
First try with the manual scope creation via the factory, I think it will work well. Let me know, thanks! |
Thank you for your thorough response @jodydonetti!
I use FusionCache in an ASP.NET web application. I do not use EF, but do use NHibernate.
In fear of running into this issue, I have completely disabled background factories.
Apologies, I should have been clearer in my initial question. Neither of these approaches are exactly what I am looking for. Creating a scope inside the factory, and only for the factory, makes it so that any scoped service will be recreated. So logic that works on assumptions that its scope is in a certain state will now fail as a completely new state is created for the factory. I might be misunderstanding the deferred scope proposal, but I am uncertain as to how it would detect future, already planned, invocations on the var t1 = queryThingAsync();
// dispose is called, but gets blocked until t1 is finished
var t2 = queryOtherThing();
await Task.WhenAll(t1, t2); If the deferred scope may only be blocked by queries that were made prior to its disposal being initiated, would this not introduce a race condition? What I am really looking for is to hang scope disposal until the factories that were invoked from the FusionCache that was "resolved" from that scope (I put resolved in quotes as FusionCache, with its singleton lifetime, would be resolved from the root, and not the scope) finish running.
I agree. My events based coordination proposal is a bandaid fix at best.
I 100% agree, I just have no way to convince ASP.NET to let me have full control of the scope.
I absolutely agree that this would be problematic if FusionCache was scope unaware as you would have no way to associate a task with the context in which it was spawned. I'm not suggesting this is easy (or even realistically possible), but as a proof-of-concept, I tried wrapping around FusionCache with the pseudo-code I wrote in this comment and accessing a scope-set wait-group. public class FusionCacheWaitGroupAccessor
{
private readonly AsyncLocal<Holder> _waitGroupHolder = new();
public FusionCacheWaitGroup? WaitGroup
{
get => _waitGroupHolder.Value?.WaitGroup;
set
{
var holder = _waitGroupHolder.Value;
if (holder is not null)
{
holder.WaitGroup = null;
}
if (value is not null)
{
_waitGroupHolder.Value = new Holder { WaitGroup = value };
}
}
}
private sealed class Holder
{
public FusionCacheWaitGroup? WaitGroup;
}
} The issue with this approach would be that you need to remember to set the value of the |
Another approach, though I have no idea how realistic it is for you, would be to somehow register a scoped I really am unsure if this is at all a viable path for you though as I don't know the inner-workings of the library enough to know how much of a pain this would be. |
Ahah, I was thinking precisely about this: FusionCache itself must be singleton, since it's a cache and the whole purpose is for it to share data globally, even among scopes. But maybe a new, lightweight thing for scopes (as a "wrapper" of sort) may be a thing. The problem I see is... what then: the whole problem with scopes is that dependencies are scoped as well (think about the How would you get into the scope disposal logic to wait for the rest? |
If your scoped service call is within the factory, you just have to convince whoever disposes the scope, in this case ASP.NET's HTTP request pipeline, to wait until the factory is done, as the factory will always finish after whatever it ran. So, for example (code might not compile, wrote off the top of my head): private readonly IScopeService _scopedService;
private readonly IScopedFusionCache _scopedFusionCache;
public IActionResult Action()
{
var result = _scopedFusionCache.GetOrSet("key", _ => _scopedService.GetThingThatWillTakeLongerThanSoftTimeout());
return Json(result);
} Now imagine a app.Use((ctx, next) =>
{
var fc = ctx.RequestServices.GetRequiredService<IScopeFusionCache>();
ctx.Response.OnCompleted(async () => await fc.WaitForBackgroundTasks());
await next(ctx);
}); This will allow the response to be sent and, before the framework looks to dispose, it will have to run Now because ASP.NET gives you the scope and disposes the scope, your |
As for non-ASP.NET HTTP request scenarios, where you create your own scope, it'd look like: using(var scope = scopeFactory.CreateScope())
{
// do your stuff
await scope.ServiceProvider.GetRequiredService<IScopedFusionCache>().WaitForBackgroundTasks();
} Preferably, you would be able to decorate Alternatively, you could look into the ordering of disposal. If you know of a way to hook onto the |
That's a lot of info, I like that 😬 Thanks for sharing! I'll have to think carefully about it and maybe make some tests, will update. |
Thank you! |
Problem
Backgrounded factories are invisible.
If your (now backgrounded) factory depends on something in the DI scope, you have no way of preventing the scope from being disposed.
This can lead your factory to fail.
Solution
When backgrounding a task in
MaybeBackgroundCompleteTimedOutFactory
, would it not be possible to alert something like "hey this task is now running in the background, heads up!".This would then allow you to prevent the scope from being disposed by doing
await fusionCache.WaitForBackgroundedTasks()
or something of the sort.I see a
Activities.EventNames.FactoryBackgroundMove
event is fired, maybe we can create & add aFusionCacheEventsHub.BackgroundFactoryMove
next to it?Alternatives
None come to mind.
Additional context
I use FusionCache on a web server and I am fine with leaving the scope open for the backgrounded tasks once an HTTP response has been issued.
I imagine putting something in
HttpContext.Response.OnCompleted
to await the background factories.The text was updated successfully, but these errors were encountered: