dazinator / Dotnettency

Mutlitenancy for dotnet applications
MIT License
111 stars 23 forks source link

`ITenantShellAccessor<TTenant>` - Restart() #1

Closed dazinator closed 5 years ago

dazinator commented 7 years ago

Consider addig a Restart method to ITenantShellAccessor<TTenant> - and can then have extensions such as the Tenant Container and Tenant Middleware Piepline, hook in to on Tenant Restarting. This way they can invalidate the transient cache for the running tenant (i.e the tenant's Container and Middleware Pipeline) - allowing them to be rebuilt on the next request for the tenant.

If anyone else writes any tenant extensions for dotnettency, they can hook into this as part of being a good citizen, honouring the intent to "restart" the particular tenant - which usually should clear any in memory state for the current tenant so they can be re-initialised on next request.

vzwick commented 7 years ago

Am I overlooking something major, or would making TenantShell IDisposable, adding a pluggable BeforeDisposeCallbacks collection and evicting the TenantShell from the ITenantShellCache after calling the callbacks and Dispose be sufficient?

dazinator commented 7 years ago

Yes I think that general idea is about right. I think the BeforeDisposeCallbacks collection could probably take the form of a CompositeDisposable of which there are numerous projects using something similar

So TenantShell would call dispose on the CompositeDisposable and that would dispose of all the registered disposables in the composite.

I think cache eviction is interesting. Yes we do need to evict the TenantShell from the cache. I think we currently use a ConcurrentDictionary for the cache implementation but my desire is for this to be swappable. However I know IMemoryCache has cache eviction policies, which might open up the option to run the disposal's as part of the cache eviction policy - meaning all we would need to do is just trigger cache eviction, and then that would trigger all the necessary disposals. This changes the flow to: Something triggers cache eviction of a particular TenantShell --> cache implementation triggeres Dispose of TenantShell (and subsequently the compositedisposable is disposed.)

Ideally, the choice of whether to use an IMemoryCache or the default ConcurrentDictionary could ultimately be surfaced in the fluent configuration API. Using an IMemoryCache means you could configured things like cache expiry time for tenants, so that tenants automatically recycled every x mins which is a nice option.

So yes, I think the mechanism is simple, but there is probably quite a bit more to it in terms of it becoming the fully fledged feature I'd like to see (and I know that probably wasn't immediately obvious from this issue) :-) what do you think?

dazinator commented 5 years ago

This is now next on my todo list. Just did a major overhaul and refactoring of dotnettency on the dev branch (2.0.0-alpha.x packages).

dazinator commented 5 years ago

I am wondering if.. when a tenantshell is disposed, should it be evicted from the tenanshellcache first (which will allow the next request to resolve and reinitialise the tenant) and then do the Disposal of services after the eviction?

Or should it do the disposal in line with the eviction - potentially blocking access for a bit longer if other requests come in. I think the latter is the safest option because I am not sure disposing of old tenant services whilst the tenant is being re-initialised on another thread is necessary a safe thing to do.. Maybe I should implement a strategy pattern and allow either style to be used, defaulting to the one I think is the safest.

dazinator commented 5 years ago

Pleased to say this is now implemented on latest develop branch and will be in the 2.0.0 release when I merge to master.