Add a fairness parameter

[samspills]

KeyPool and Pool now support a fairness parameter that controls how incoming requests are handled.
When fairness is true requests are served in FIFO order, and when fairness is false requests are served in LIFO order.

The default value here is true, this is the opposite of commons-pool2 but it maintains consistency for people using keypool so I think it's the correct choice.

The behaviour switch is handled by a custom semaphore that we're using now, instead of the existing cats-effect semaphore. That semaphore has a BackingQueue that can switch behaviour between FIFO and LIFO (this is toggled using the fairness parameter)

[rossabaker]

This is my fault for holding up commons-pool2 as an idol, but I am scared of boolean blindness here. What if we had Fairness.Lifo and Fairness.Fifo instead?

[samspills]

That's a good idea, and I've made the update.

I left it just as Fairness so far, but every time we've been talking about this we've had to be careful to make sure we were all talking about request ordering and not the ordering when grabbing connections from the pool. Do you think Fairness is something everyone will understand or should it be more obvious and renamed to something like RequestFairness?

[rossabaker]

I think my confusion has stemmed from fairness as being about requests, but thinking of Lifo/Fifo as being about the leasing of idle objects ... even though Lifo/Fifo is about both. I'd probably rather change the name of the idle object order from commons-pool2 and leave this one alone.

[samspills]

👍🏻 That makes sense to me

[NthPortal]

does Typelevel really need to explicitly license code from different projects to each other?

[samspills]

I'm not 100% sure, but especially since keypool and cats-effect are licensed differently I thought so (I would defer to @rossabaker's wisdom here)

[rossabaker]

If I didn't write it, and I copy it into a new project, I credit it, even if I'm a maintainer on both projects. It's probably not strictly necessary in this case, but once we play fast and loose with attribution, it's a slippery slope toward working for Microsoft¹.

Footnotes

1. fuckers ↩

[danicheg]

LGTM, just a few unsolicited nits that can be freely ignored 👍🏻

[danicheg]

It's just a nit but I'm struggling to understand what Request stands for in terms of Semaphore.

Additionally it takes a [[Fairness]] parameter, used to toggle the order in which requests acquire a permit.

Maybe it's more likely DirectionalSemaphore but it reveals implementation details though...

[samspills]

Yeah, I struggled a lot with naming here. It's a RequestSemaphore in the sense that it is the Semaphore that controls how Requests for connections are processed through keypool. It's used in the take function and is what blocks on taking a connection from the pool if one isn't available.