Add ability to expire resources within the pool #216
Conversation
Structs are passed by value meaning that the last_checked_out recorded never actually gets updated
Oops
The number of inflight resources was not properly incremented or compared against resulting in the possibility of fibers queuing up many unnecessary replenishments to the pool.
Allow integer arguments for max expiration time Allow using `Time::Span` for the resource sweeper timer
Replace call to Time::Span.positive? Bumps some tiny sleep duration to safeguard test succcess
Although handled by `PoolResourceLost(T)` doing so manually avoids a mutex lock and a second redundant call to #delete
| elsif can_increase_idle_pool | ||
| # Checks lifetime expiration and updates last checked out time | ||
| # Old idle expiration isn't checked because this replaces it. | ||
| expire_info = @resource_lifecycle[resource] |
There was a problem hiding this comment.
This is also technically a breaking change in that it'll now raise when a non pool item is released into the pool
|
|
||
| return if replenish <= 0 | ||
|
|
||
| replenish.times do |index| |
There was a problem hiding this comment.
The resource replenishment could actually block a checkout/release operation for a really long time depending on how many resources it needs to be built. In most cases it should just be a single one but it can quickly balloon when many resources are discarded manually from outside of the pool. I'm not entirely sure what's the best way to resolve this.
I feel like spawning a new fiber each time is excessive, so maybe it could be dispatched to the already existing background job?
There's also the fact that any non-blocking operation will remove the guarantee of there always being at least initial_pool_size fresh resources available...
Closes #47
This PR implements the ability to expire assets within the pool based on what I think was discussed in #47
This is configured through the two new pool parameters
max_lifetime_per_resourceandmax_idle_time_per_resourcewhich sets the number of seconds before they get expired and removed from the pool. This is done on each#checkoutand#releasebut also with an optional background fiber which runs on either a configured timer, or whichever configured expiration is shorter.This PR mostly also respects
initial_pool_sizein that it'll try to keep at least that many resources within the pool--but it doesn't actually know which resources are stale until it checks on a#checkout/#releaseor with the background fiber so they're not necessarily always fresh.But if any of the expiration check causes the amount of resources within pool to drop below
initial_pool_sizeit'll immediately replenish with fresh resources till its back up toinitial_pool_size.Also when a
#checkoutinternally finds an expired resource, the newPoolResourceExpiredexception is raised. So#retrywill get more important.The specs all seems to pass and modifying the manual specs to use the new expiration features did not show any glaring problems either.
Please let me know if I've missed something with this implementation! I wrote this over the course of a single night so apologies in advance if the code might be a bit messy 😅