TryAcquire will be blocked when construct resource failed about puddle HOT 11 CLOSED

It is behaving as documented.

TryAcquire is the same as Acquire except it returns ErrNotAvailable if the pool is at maximum capacity and no resources are available.

You can use provided context to limit how long to wait. (e.g. with https://pkg.go.dev/net#Dialer.DialContext)

from puddle.

I expected the tcp network to remain blocked for 5 seconds, but TryAquirue would return immediately for whatever reason.

from puddle.

So that would be something like always return immediately if nothing is immediately available but start a background acquire if there is room in the pool. Hopefully, the resource would be available next time around. Maybe even return different errors to distinguish between pool is empty and cannot grow and starting a background acquire.

That could be a reasonable change. But @ajjensen13 added this feature just a little over a month ago. I'd like to get their opinion if possible.

from puddle.

I don't have a strong opinion either way.

Although, whether or not the constructor blocks, and for how long, is outside the control of this library. Which makes it feel out of place to code around the assumption that a constructor blocks.

I have no opposition to the proposal. But, might suggest investigating a wrapper function that calls TryAcquire in a go-routine to handle this specific case

from puddle.

The main reason is that the current resource management is synchronous. If we need to implement TryAcquire, it is non-blocking in any situation. We need to complete the creation of resources in the goroutine, and automatically create supplements if the resources are insufficient. If there are free resources, return immediately. If the total number of resources is zero, TryAcquire immediately returns the reason why the resource is unavailable or the last resource creation failed.

from puddle.

I created PR #15 that implements most of what we have discussed. The only thing I didn't do was separate errors for why the TryAcquire failed. There was some ambiguity regarding what error message would be appropriate in certain circumstances. These can be added later if necessary.

Let me know if this works for everyone.

from puddle.

Thank you for your PR, this PR can meet my needs and it is great. But repeatedly calling TryAcquire may cause the resources in the pool to exceed the maximum number, and we can think about how to return an error.

from puddle.

But repeatedly calling TryAcquire may cause the resources in the pool to exceed the maximum number,

🤦 Wow. Can't believe I missed that. I guess that's why it's helpful to have some eyes on concurrent code. Just pushed a fix that should prevent the pool from overfilling.

from puddle.

I reviewed the code and I think there is no problem, thank you.

from puddle.

Hello, may I ask, when can you merge the PR into the master branch?

from puddle.

Done. I wanted to wait a few days to see if anyone else wanted to comment / review and to give myself a chance to look at it with fresh eyes. It still appeared correct to me. Merged and tagged v1.2.1.

from puddle.