queue is not cleared immediately after client timeout

[aymanosman]

It seems that state.queue is not immediately cleared (while state.requests is) after a client times out.

defmodule BadPool do
  @behaviour NimblePool

  @doc """

  """
  def start_link(_args \\ []) do
    NimblePool.start_link(
      worker: {__MODULE__, nil},
      pool_size: 1,
      name: __MODULE__
    )
  end

  def command(sleep \\ 5_000) do
    NimblePool.checkout!(
      __MODULE__,
      :checkout,
      fn _, _ ->
        Process.sleep(sleep)
      end,
      1000
    )
  end

  @impl NimblePool
  def init_worker(_) do
    {:ok, nil, nil}
  end

  @impl NimblePool
  def handle_checkout(:checkout, _, _, _) do
    {:ok, nil, nil, nil}
  end
end

It seems the queue will build up until the connection is checked back in.

> {:ok, pool} = BadPool.start_link()
> Task.start(fn -> BadPool.command(60_000) end) # do this a few times
> :sys.get_state(pool)
%{
  async: %{},
  lazy: nil,
  max_idle_pings: -1,
  monitors: %{
    #Reference<0.33670699.3888644103.189962> => #Reference<0.33670699.3888644103.189961>
  },
  # queue builds up despite...
  queue: {[
     {#PID<0.238.0>, #Reference<0.33670699.3888644097.191297>},
     {#PID<0.236.0>, #Reference<0.33670699.3888644097.191289>},
     {#PID<0.234.0>, #Reference<0.33670699.3888644097.191281>},
     {#PID<0.232.0>, #Reference<0.33670699.3888644097.191273>}
   ], [{#PID<0.230.0>, #Reference<0.33670699.3888644097.191265>}]},
 # ... timed-out requests having been removed here
  requests: %{
    #Reference<0.33670699.3888644103.189961> => {#PID<0.227.0>,
     #Reference<0.33670699.3888644103.189962>, :state, nil}
  },
  resources: {[], []},
  state: nil,
  worker: BadPool,
  worker_idle_timeout: nil
}

I tried the same thing with a pool size of 2 with two initial requests, one with a sleep of "infinity" and another for 60 seconds. The pool cleared as soon as the 60 second request finished.

So it seems that if all connections are checked out, the queue will grow indefinitely despite clients timing out.

I was just wondering if this is the desired behaviour?

[josevalim]

Yes, this is expected because looking up the queue to remove requests is a linear operation, which would be expensive. So we keep the queue entries lightweight and we prune the time we need to checkout next.