Open jebrosen opened 4 years ago
There's a number of ways to implement this concept, but I'm not sure which is most ideal. A few of my thoughts:
AsyncPoolGuard::mark_broken
function that simply drops the resource and signals for the creation of a new one from a registered factoryAsyncPoolGuard::recycle
function which drops the resource and immediately creates a new one (maybe with a recycle_with
that overrides the registered factory)tokio_resource_pool
's Manage
A callable function on the guard makes it the responsibility of the user of the guard to check for or deal with brokenness, while putting it on the trait makes it the responsibility of the implementer of the trait. It's possible to implement an abstraction over the first that works like the second; I have not put much thought into the other way around. For one of my use cases (i.e. "could Rocket use async_pool
"), the trait is more convenient but guard methods should be usable.
There is also the issue of when brokenness is checked: a connection that idles long enough could become broken due to that, at which point it needs to be recycled upon being retrieved. I have not looked at when which libraries do their respective checks, but several of them have both a "fast check" ("definitely broken" vs undetermined) and a "slow check" ("definitely usable" vs not).
Of course, "brokenness" is a property that applies to and is necessary to handle resources such as database connections or file handles but not might apply to other resources that are worth pooling. I opened this issue with databases in mind, but this library looks to be geared towards more general use than databases.
The compiler is smart enough to optimize out calls to default method bodies on traits that are empty. So, despite this being general purpose, a trait could still make for no overhead for more simple resources.
I'm starting to lean towards using the trait, only. An is_sound
function (slow check) on the Manager trait called on rsvp, and an is_broken
function (fast check) on guard drop. Make it the pool's responsibility to ensure sound resources.
Question this brings up is if I should require the Manager trait to replace broken resources, or if I should return an error on rsvp when the count hits 0. I don't like the latter, but I'm not sure if the former is reasonable.
Other pools have a way to signal that a resource is "broken" and a fresh connection is needed:
r2d2
:has_broken
bb8
:has_broken
tokio_resource_pool
:recycle
I believe this functionality can not be implemented on top of this crate's current API because there's no way to remove a resource from the pool, e.g. with a
remove()
function orinto_inner()
which would prevent the resource from being returned to the pool.