general-cbic / poolex Goto Github PK

• Describe behaviours
• Add and use implementations
• Add the ability to configure implementations

Feature inspired by poolboy.

Poolex should be able to create a configurable number of workers in excess of a constant value (if the load has increased and there are no free workers left in the pool).

Hello!

It seems like the library doesn't handle the shutdown process. The pool dies but the workers are still alive, which is very inconvenient. I took a quick look at poolboy and it uses a supervisor where the workers are placed to handle the shutdown process correctly. Do you have a plan to do something similar?

Thank you!

It's inconvenient to describe the spec manually :(

Poolex's tests must cover cases of handling sudden death of workers. The current implementation has crash logs and error messages while tests are running.

There is no big problem, but it will be nice to make logs clean.

Example of the test pipeline:

Hi!

When the function you use in Poolex.run raise a timeout, it returns :all_workers_are_busy. I did a test alexcastano@c84a84b

If I have time, I will finish the PR with the solution. However, I think the timeout option can be misleading. I expected to be the timeout for the whole process, but it is only used to get the worker. The documentation is correct, but you have to check the documentation: https://hexdocs.pm/poolex/Poolex.html#t:run_option/0. Maybe we can find a better name to avoid confusion, like :checkout_timeout?

Ecto used to have :pool_timeout, but it was deprecated. In Ecto :timeout includes the queue time and the query time: https://hexdocs.pm/ecto/changelog.html#backwards-incompatible-changes-1 . This was my expectation for poolex too :)

Thanks

I didn't create a test, but I think when a worker dies Poolex doesn't verify if there are new callers waiting for a worker:
https://github.com/general-CbIC/poolex/blob/main/lib/poolex.ex#L449-L482

We do here for example:
https://github.com/general-CbIC/poolex/blob/main/lib/poolex.ex#L401

I also think that Poolex wouldn't need to make a difference between a "temporary worker" and a "regular worker", and it could make the code easier (less complex). We only need to verify the number the workers and the configuration params:

If a worker dies, we remove it from the list of workers, and then:

• We check if there are pending callers, we create a new worker
• If not we check if the number of current workers is less than minimum, then we create a new worker.
• Else, we remove don't create a new one.

Similarly, when a worker is released:

• If we have pending callers, we send the worker to the caller
• If the number of the current workers is in overflow, we kill it.
• If not, we put it on the idle list.

I hope it helps

Let's push it via Github actions

One thing has been lacking from poolboy and it'd be really cool to see it make its way to Poolex.
It's the ability to manage workers as "groups" per pool.

For example:

1. Being able to scale down a pool to zero workers and back to another number.
2. Being able to checkout all workers at once and send them a message.
3. Being able to introspect a pool and get the workers PIDs.

Using Poolboy, we were achieving 1. by calling :poolboy.stop(:pool_id), and for example this function is missing for Poolex.

Poolex.State.t() not found during documentation generation.

❯ mix docs                                                                                                                                                                                                                                 
Generating docs...
warning: documentation references type "Poolex.State.t()" but it is hidden or private
  lib/poolex.ex:178: Poolex.get_state/1

View "html" docs at "doc/index.html"
warning: documentation references type "Poolex.State.t()" but it is hidden or private
  lib/poolex.ex:178: Poolex.get_state/1

View "epub" docs at "doc/poolex.epub"

• run/3 will return {:error, reason :: atom()} on errors
• run!/3 will raise on errors

Reasons are described here.

Currently implementations are configured via config :poolex, .... We need to get away from this in favor of initialization parameters.