elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
309 stars 112 forks source link

`ping` fires every other `:idle_interval`, rather than every `:idle_interval` #243

Closed daducharmehg closed 3 years ago

daducharmehg commented 3 years ago

Issue description

Noticed this while adjusting my project's env specific :idle_interval values. When a connection is idle, pings only fire half as often as I expect them to fire. I have reproduced this on versions ~> 2.2 and ~> 2.4.

Steps to reproduce the issue

With the following options being passed:

IDLE_INTERVAL=5000
POOL_SIZE=1

and a ping function of:

@spec ping(state :: any()) ::
          {:ok, new_state :: any()}
          | {:disconnect, Exception.t(), new_state :: any()}
  def ping(state) do
    # This runs the query against our DB 
    query = %Snowpack.Query{name: "ping", statement: "SELECT /* heartbeat */ 1;"}

    case do_query(query, [], [], state) do
      {:ok, _, new_state} -> {:ok, new_state}
      {:error, reason, new_state} -> {:disconnect, reason, new_state}
      other -> other
    end

With no queries being run (the single conn in the pool is idle), I see "SELECT /* heartbeat */ 1;" in my DB's logs every 10s.

What's the expected result?

Ping should fire once every 5 seconds, IE once every idle interval

What's the actual result?

Ping fires every other idle interval.

Additional details / screenshot

Let me know if you agree that this is a bug. I suspect that the query being run in the ping is causing the next idle interval check to detect the conn as ready. This is inconvenient from a user perspective as if I have a fixed connection timeout, I need to set my idle interval to half of that value to actually keep it alive. This is unintuitive.

josevalim commented 3 years ago

I checked the code and the idle_interval controls how frequently we go through the pool. So in practice, your connections will be pinged between idle_interval <= time < 2 * idle_interval, given enough time or enough usage, you will see it phased out a bit. If you need more precise timing, then you can embed it directly in your connection process and disable the pool idle_interval, especially because we may add pool specific features in the future, such as randomization, which you likely don't want if you have tight constraints. I will improve the docs meanwhile. Thanks!