Use the connection pool in ownership and remove other pools (#130)

* Remove ownership pool and always use the connection pool

* Remove poolboy and connection pools

README.md

@@ -4,10 +4,6 @@ Database connection behaviour and database connection pool designed for
 handling transaction, prepare/execute, cursors and client process
 describe/encode/decode.
 
-Three pool implementations are provided: `DBConnection.Connection`
-(default/single connection), `DBConnection.Poolboy` (poolboy pool)
-and `DBConnection.Ownership` (ownership pool).
-
 Examples of using the `DBConnection` behaviour are available in
 `./examples/db_agent/` and `./examples/tcp_connection/`.
 

examples/db_agent/lib/db_agent.ex

@@ -1,16 +1,13 @@
 defmodule DBAgent do
-
   use DBConnection
 
-
   defmodule Query do
     defstruct [:query]
   end
 
   @spec start_link((() -> state :: any), Keyword.t) :: GenServer.on_start
   def start_link(fun, opts \\ []) when is_function(fun, 0) do
-    opts = [init: fun, pool: DBConnection.Connection, sync_connect: true,
-            backoff: nil] ++ opts
+    opts = [init: fun, backoff: nil] ++ opts
     DBConnection.start_link(__MODULE__, opts)
   end
 

examples/db_agent/mix.lock

@@ -1,3 +1 @@
-%{"backoff": {:hex, :backoff, "1.1.1"},
-  "connection": {:hex, :connection, "1.0.2"},
-  "poolboy": {:hex, :poolboy, "1.5.1"}}
+%{"connection": {:hex, :connection, "1.0.2"}}

integration_test/cases/queue_test.exs

@@ -108,23 +108,6 @@ defmodule QueueTest do
     assert P.run(pool, fn(_) -> :result end) == :result
   end
 
-  @tag :enqueue_disconnected
-  test "queue raises disconnect error when disconnected" do
-    stack = [{:error, RuntimeError.exception("oops")}]
-    {:ok, agent} = A.start_link(stack)
-
-    opts = [agent: agent, parent: self(), backoff_start: 30_000]
-    {:ok, pool} = P.start_link(opts)
-
-    {queue_time, _} = :timer.tc(fn() ->
-      opts = [queue: false]
-      assert_raise DBConnection.ConnectionError,
-      "connection not available because of disconnection",
-        fn() -> P.run(pool, fn(_) -> flunk("got connection") end, opts) end
-    end)
-    assert queue_time <= 1_000_000, "request was queued"
-  end
-
   @tag :dequeue_disconnected
   test "queue raises dropped from queue when disconnected" do
     stack = [{:error, RuntimeError.exception("oops")}]

integration_test/connection_pool/test_helper.exs

@@ -1,11 +1,7 @@
-ExUnit.start([capture_log: true, assert_receive_timeout: 500,
-              exclude: [:pool_overflow, :enqueue_disconnected,
-                        :queue_timeout_exit]])
+ExUnit.start(capture_log: true, assert_receive_timeout: 500, exclude: [:queue_timeout_exit])
 
-Code.require_file "../../test/test_support.exs", __DIR__
+Code.require_file("../../test/test_support.exs", __DIR__)
 
 defmodule TestPool do
-  use TestConnection, [pool: DBConnection.ConnectionPool, pool_size: 1]
+  use TestConnection, pool: DBConnection.ConnectionPool, pool_size: 1
 end
-
-{:ok, _} = TestPool.ensure_all_started()

integration_test/ownership/owner_test.exs

@@ -5,25 +5,6 @@ defmodule OwnerTest do
   alias TestAgent, as: A
   alias DBConnection.Ownership
 
-  defmodule BadPool do
-    def checkout(_, _) do
-      {:error, DBConnection.ConnectionError.exception("connection not available")}
-    end
-  end
-
-  test "allows a custom pool than the started one on checkout" do
-    {:ok, pool} = start_pool()
-
-    assert Ownership.ownership_checkout(pool, [ownership_pool: UnknownPool]) ==
-      {:error, %DBConnection.ConnectionError{message: "failed to checkout using UnknownPool"}}
-  end
-
-  test "returns error on checkout" do
-    {:ok, pool} = start_pool()
-    assert {:error, %DBConnection.ConnectionError{}} =
-      Ownership.ownership_checkout(pool, [ownership_pool: BadPool])
-  end
-
   test "reconnects when owner exits during a client checkout" do
     stack = [
       {:ok, :state},
@@ -59,15 +40,7 @@ defmodule OwnerTest do
       {:connect, _}] = A.record(agent)
   end
 
-  defp start_pool do
-    stack = [{:ok, :state}]
-    {:ok, agent} = A.start_link(stack)
-
-    opts = [agent: agent, parent: self()]
-    P.start_link(opts)
-  end
-
-  test "reconnect when ownership times out" do
+  test "reconnects when ownership times out" do
     stack = [
       {:ok, :state},
       :ok,

integration_test/ownership/shutdown_test.exs

@@ -11,15 +11,13 @@ defmodule TestShutdown do
     opts = [agent: agent, parent: self(), shutdown: :brutal_kill]
     {:ok, pool} = P.start_link(opts)
 
-    assert {caller_refs, started_refs} = :sys.get_state(DBConnection.Watcher)
-    caller_ref = Enum.find_value(started_refs, fn {_, {pid, ref}} -> pid == pool && ref end)
-    {_, pool_sup, _} = Map.fetch!(caller_refs, caller_ref)
-
-    monitor = Process.monitor(pool_sup)
-    _ = Process.flag(:trap_exit, true)
+    {:links, links} = Process.info(pool, :links)
+    [inner_pool] = links -- [self()]
 
+    Process.flag(:trap_exit, true)
+    ref = Process.monitor(inner_pool)
     Process.exit(pool, :shutdown)
-    assert_receive {:DOWN, ^monitor, _, _, :shutdown}
+    assert_receive {:DOWN, ^ref, _, _, :shutdown}
 
     assert [connect: [_]] = A.record(agent)
   end
@@ -31,15 +29,12 @@ defmodule TestShutdown do
     opts = [agent: agent, parent: self(), shutdown: :brutal_kill]
     {:ok, pool} = P.start_link(opts)
 
-    assert {caller_refs, started_refs} = :sys.get_state(DBConnection.Watcher)
-    caller_ref = Enum.find_value(started_refs, fn {_, {pid, ref}} -> pid == pool && ref end)
-    {_, pool_sup, _} = Map.fetch!(caller_refs, caller_ref)
+    {:links, links} = Process.info(pool, :links)
+    [inner_pool] = links -- [self()]
 
     _ = Process.flag(:trap_exit, true)
-    sup = DBConnection.Ownership.PoolSupervisor
-    assert Supervisor.terminate_child(sup, pool_sup) == :ok
-    assert_receive {:EXIT, ^pool, :killed}
-
+    Process.exit(inner_pool, :shutdown)
+    assert_receive {:EXIT, ^pool, :shutdown}
     assert [connect: [_]] = A.record(agent)
   end
 end

integration_test/ownership/test_helper.exs

@@ -1,11 +1,11 @@
-ExUnit.start([capture_log: true, assert_receive_timeout: 500,
-              exclude: [:idle_timeout, :pool_overflow, :dequeue_disconnected,
-                        :queue_timeout_raise]])
+ExUnit.start(
+  capture_log: true,
+  assert_receive_timeout: 500,
+  exclude: [:idle_timeout, :queue_timeout_raise]
+)
 
-Code.require_file "../../test/test_support.exs", __DIR__
+Code.require_file("../../test/test_support.exs", __DIR__)
 
 defmodule TestPool do
-  use TestConnection, [pool: DBConnection.Ownership, pool_size: 1]
+  use TestConnection, pool: DBConnection.Ownership, pool_size: 1
 end
-
-{:ok, _} = TestPool.ensure_all_started()

integration_test/tests.exs

@@ -5,7 +5,6 @@ Code.require_file "cases/close_test.exs", __DIR__
 Code.require_file "cases/cursor_test.exs", __DIR__
 Code.require_file "cases/execute_test.exs", __DIR__
 Code.require_file "cases/idle_test.exs", __DIR__
-Code.require_file "cases/overflow_test.exs", __DIR__
 Code.require_file "cases/prepare_execute_test.exs", __DIR__
 Code.require_file "cases/prepare_stream_test.exs", __DIR__
 Code.require_file "cases/prepare_test.exs", __DIR__

lib/db_connection.ex

@@ -347,32 +347,11 @@ defmodule DBConnection do
     end
   end
 
-  @doc """
-  Ensures the given pool applications have been started.
-
-  ### Options
-
-    * `:pool` - The `DBConnection.Pool` module to use, (default:
-    `DBConnection.Connection`)
-
-  """
-  @spec ensure_all_started(opts :: Keyword.t, type :: atom) ::
-    {:ok, [atom]} | {:error, atom}
-  def ensure_all_started(opts, type \\ :temporary) do
-    Keyword.get(opts, :pool, DBConnection.Connection).ensure_all_started(opts, type)
-  end
-
   @doc """
   Start and link to a database connection process.
 
   ### Options
 
-    * `:pool` - The `DBConnection.Pool` module to use, (default:
-    `DBConnection.Connection`)
-    * `:idle` - The idle strategy, `:passive` to avoid checkin when idle and
-    `:active` to checkin when idle (default: `:passive`)
-    * `:idle_timeout` - The idle timeout to ping the database (default:
-    `1_000`)
     * `:backoff_min` - The minimum backoff interval (default: `1_000`)
     * `:backoff_max` - The maximum backoff interval (default: `30_000`)
     * `:backoff_type` - The backoff strategy, `:stop` for no backoff and
@@ -394,7 +373,7 @@ defmodule DBConnection do
   """
   @spec start_link(module, opts :: Keyword.t) :: GenServer.on_start
   def start_link(conn_mod, opts) do
-    pool_mod = Keyword.get(opts, :pool, DBConnection.Connection)
+    pool_mod = Keyword.get(opts, :pool, DBConnection.ConnectionPool)
     apply(pool_mod, :start_link, [conn_mod, opts])
   end
 
@@ -406,7 +385,7 @@ defmodule DBConnection do
   @spec child_spec(module, opts :: Keyword.t, child_opts :: Keyword.t) ::
     Supervisor.Spec.spec
   def child_spec(conn_mod, opts, child_opts \\ []) do
-    pool_mod = Keyword.get(opts, :pool, DBConnection.Connection)
+    pool_mod = Keyword.get(opts, :pool, DBConnection.ConnectionPool)
     apply(pool_mod, :child_spec, [conn_mod, opts, child_opts])
   end
 
@@ -1338,7 +1317,7 @@ defmodule DBConnection do
 
   defp checkout(pool, meter, opts) do
     meter = event(meter, :checkout)
-    pool_mod = Keyword.get(opts, :pool, DBConnection.Connection)
+    pool_mod = Keyword.get(opts, :pool, DBConnection.ConnectionPool)
     try do
       apply(pool_mod, :checkout, [pool, opts])
     catch

lib/db_connection/app.ex

@@ -7,7 +7,7 @@ defmodule DBConnection.App do
   def start(_, _) do
     children = [
       supervisor(DBConnection.Task, []),
-      supervisor(DBConnection.Ownership.PoolSupervisor, []),
+      supervisor(DBConnection.Ownership.ProxySupervisor, []),
       supervisor(DBConnection.ConnectionPool.PoolSupervisor, []),
       worker(DBConnection.Watcher, [])
     ]