Ownership timeout cannot be configured

[myronmarston]

According to the Ecto docs, the :ownership_timeout can be configured in config.exs:

@moduledoc """
  # ...
  If you have a long running test (or you're debugging with IEx.pry), the timeout for the connection ownership may
  be too short.  You can increase the timeout by setting the
  `:ownership_timeout` options for your repo config in `config/config.exs` (or preferably in `config/test.exs`):

      config :my_app, MyApp.Repo,
        ownership_timeout: NEW_TIMEOUT_IN_MILLISECONDS

  The `:ownership_timeout` option is part of
  [`DBConnection.Ownership`](https://hexdocs.pm/db_connection/DBConnection.Ownership.html)
  and defaults to 15000ms. Timeouts are given as integers in milliseconds.
  # ...
"""

I tried this and could not get it to work. Looking at DBConnection.Ownership.Proxy.start_link/4 and DBConnection.Ownership.Proxy.init/2, I can see why:

  def start_link(manager, caller, pool, pool_opts) do
    GenServer.start_link(__MODULE__, {manager, caller, pool, pool_opts}, [])
  end

  def init(proxy, opts) do
    ownership_timeout = opts[:ownership_timeout] || @ownership_timeout
    case GenServer.call(proxy, {:init, ownership_timeout}, :infinity) do
      :ok                 -> :ok
      {:error, _} = error -> error
   end
  end

start_link/4 is hardcoded to pass [] as the opts to init/2.

At a minimum, it would be great if it could be configured via config.exs as the Ecto docs state. In addition, I would love it if there was a way to configure it for a particular ownership checkout. 15 seconds is reasonable in general for my test suite (excluding a handful of smoke tests I don't usually run locally, my suite runs over 2K tests in 30 seconds) but for the smoke tests--which actually hit S3 to fetch data instead of using a test fake--it can take longer than 15 seconds we often hit the timeout. I would like to be able to pass an alternate timeout in the Ecto.Adapters.SQL.Sandbox.checkout call (or in Ecto.Adapters.SQL.Sandbox.mode call, if that makes more sense) so that the longer timeout is only used for a particular test.

[fishcakez]

The opts to init/2 come from here: https://github.com/elixir-ecto/db_connection/blob/95e14fe05202c4e47fd63151de0773a9d85ea8b6/lib/db_connection/ownership.ex#L59. So it is possible to pass a custom :ownership_timeout (and other options) but Ecto only passes the compile time configuration: https://github.com/elixir-ecto/ecto/blob/2d41c5b5c0597def6cee0ade6eb39f922d5663fe/lib/ecto/adapters/sql/sandbox.ex#L454. Therefore we want to add ownership_timeout to that function in Ecto. Would you like to do a PR?

[myronmarston]

The opts to init/2 come from here: https://github.com/elixir-ecto/db_connection/blob/95e14fe05202c4e47fd63151de0773a9d85ea8b6/lib/db_connection/ownership.ex#L59.

Oh right. I was thinking that the GenServer callback was init/1, not init/2 and was quite confused by the fact that it seemed different here...

Anyhow, after spending quite a long time troubleshooting it, I've discovered that the reason I couldn't get the option to work from config.exs is because it gets compiled into the repo's __pool__/0 function. To get my config change to "take" I have to rm -rf _build.

Should Elixir automatically re-compile the repo module when my config changes? The docs didn't say anything (from what read, at least) about a need to force recompilation so I frankly wasn't expecting to have to do that.

Therefore we want to add ownership_timeout to that function in Ecto. Would you like to do a PR?

Sure! It might be a few days before I get to it.

[fishcakez]

@myronmarston The best place to look for configuration docs is the adapters page, e.g. https://github.com/elixir-ecto/ecto/blob/5e4a8499aac4fbf60d06fae04bf29600af9af650/lib/ecto/adapters/postgres.ex#L24, and :ownership_timeout is a pool: Ecto.Adapters.Sandbox.SQL option, so also compile time. For future there is mix compile --force to do a recompile.