Skip to content

Conversation

@iRevive
Copy link
Contributor

@iRevive iRevive commented Nov 10, 2021

My attempt to fix #291

}
}
case PoolOpen(_, borrowed, m2) =>
borrowed.toList.traverse { case (_, (_, destroy)) => destroy.attempt.void } >>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to invoke a user-specified callback in such an unusual scenario.

E.g.

class Builder[F[_]: Temporal, A, B] private[keypool] {
  ...
  def withOnBorrowedTermination(f: List[(A, F[Unit])] => F[Unit]): Builder
}

pm match {
case PoolClosed() => (0, Map.empty)
case PoolOpen(idleCount, m) =>
case PoolOpen(idleCount, _, m) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, a number of borrowed connections can be a part of the exposed state. It's a binary breaking change (if we talk about KeyPool#state), but this data can be made available via another method.

Moreover, if represent a borrowed resource as (B, F[Unit], FiniteDuration), leaking resources can be spotted.

@rossabaker rossabaker self-requested a review November 14, 2021 02:02
@ChristopherDavenport
Copy link
Member

I've been working on something that takes this to a much further degree. The one major takeaway I've had is that you don't want this to be always true. It seems in line with the current implementation though. But this current implementation needs a lot more power to handle even more complex pooling scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A borrowed resource can postpone its termination

2 participants