From 471f19073c6f039a3a76a7532205878c1b9c30b7 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Tue, 24 Oct 2017 18:10:00 -0700 Subject: [PATCH 1/9] only hibernate when backoff and timeout --- lib/db_connection/connection.ex | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index 708a2c5..6bec3bc 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -113,7 +113,8 @@ defmodule DBConnection.Connection do after_connect_timeout: Keyword.get(opts, :after_connect_timeout, @timeout), idle: idle, idle_timeout: Keyword.get(opts, :idle_timeout, @idle_timeout), - idle_time: 0, after_timeout: after_timeout} + idle_time: 0, after_timeout: after_timeout, + hibernate?: Application.get_env(:db_connection, :enable_hibernate, false)} if mode == :connection and Keyword.get(opts, :sync_connect, false) do connect(:init, s) else @@ -130,7 +131,7 @@ defmodule DBConnection.Connection do def connect(_, s) do %{mod: mod, opts: opts, backoff: backoff, after_connect: after_connect, idle: idle, idle_timeout: idle_timeout, regulator: regulator, - lock: lock} = s + lock: lock, hibernate?: hibernate?} = s case apply(mod, :connect, [opts]) do {:ok, state} when after_connect != nil -> ref = make_ref() @@ -153,7 +154,7 @@ defmodule DBConnection.Connection do end) done_lock(regulator, lock) {timeout, backoff} = Backoff.backoff(backoff) - {:backoff, timeout, %{s | lock: nil, backoff: backoff}} + maybe_hibernate(hibernate?, {:backoff, timeout, %{s | lock: nil, backoff: backoff}}) end end @@ -377,6 +378,9 @@ defmodule DBConnection.Connection do def handle_info(msg, %{client: {_, :connect}} = s) do do_handle_info(msg, s) end + def handle_info(:timeout, %{hibernate?: hibernate?} = s) do + maybe_hibernate(hibernate?, {:noreply, s}) + end def handle_info(msg, %{mod: mod} = s) do Logger.info(fn() -> [inspect(mod), ?\s, ?(, inspect(self()), ") missed message: " | @@ -563,10 +567,17 @@ defmodule DBConnection.Connection do end end - defp handle_timeout(%{client: nil, idle_timeout: idle_timeout} = s) do - {:noreply, s, idle_timeout} + defp handle_timeout(%{client: nil, idle_timeout: idle_timeout, + hibernate?: hibernate?} = s) do + Process.send_after(self(), :timeout, idle_timeout) + maybe_hibernate(hibernate?, {:noreply, s}) + end + defp handle_timeout(%{hibernate?: hibernate?} = s) do + maybe_hibernate(hibernate?, {:noreply, s}) end - defp handle_timeout(s), do: {:noreply, s} + + defp maybe_hibernate(true, res), do: Tuple.append(res, :hibernate) + defp maybe_hibernate(_, res), do: res defp demonitor({_, mon}) when is_reference(mon) do Process.demonitor(mon, [:flush]) From 2e065e6fd5389bc9fedc17c1681506cd82c57cbe Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Mon, 20 Nov 2017 15:40:52 +0800 Subject: [PATCH 2/9] use idle_hibernate as pure options --- lib/db_connection/connection.ex | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index 6bec3bc..5b73fb5 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -18,9 +18,10 @@ defmodule DBConnection.Connection do require Logger alias DBConnection.Backoff - @pool_timeout 5_000 - @timeout 15_000 - @idle_timeout 1_000 + @pool_timeout 5_000 + @timeout 15_000 + @idle_timeout 1_000 + @idle_hibernate false ## DBConnection.Pool API @@ -114,7 +115,7 @@ defmodule DBConnection.Connection do @timeout), idle: idle, idle_timeout: Keyword.get(opts, :idle_timeout, @idle_timeout), idle_time: 0, after_timeout: after_timeout, - hibernate?: Application.get_env(:db_connection, :enable_hibernate, false)} + idle_hibernate: Keyword.get(opts, :idle_hibernate, @idle_hibernate)} if mode == :connection and Keyword.get(opts, :sync_connect, false) do connect(:init, s) else @@ -131,7 +132,7 @@ defmodule DBConnection.Connection do def connect(_, s) do %{mod: mod, opts: opts, backoff: backoff, after_connect: after_connect, idle: idle, idle_timeout: idle_timeout, regulator: regulator, - lock: lock, hibernate?: hibernate?} = s + lock: lock, idle_hibernate: idle_hibernate} = s case apply(mod, :connect, [opts]) do {:ok, state} when after_connect != nil -> ref = make_ref() @@ -154,7 +155,7 @@ defmodule DBConnection.Connection do end) done_lock(regulator, lock) {timeout, backoff} = Backoff.backoff(backoff) - maybe_hibernate(hibernate?, {:backoff, timeout, %{s | lock: nil, backoff: backoff}}) + maybe_hibernate(idle_hibernate, {:backoff, timeout, %{s | lock: nil, backoff: backoff}}) end end @@ -378,8 +379,8 @@ defmodule DBConnection.Connection do def handle_info(msg, %{client: {_, :connect}} = s) do do_handle_info(msg, s) end - def handle_info(:timeout, %{hibernate?: hibernate?} = s) do - maybe_hibernate(hibernate?, {:noreply, s}) + def handle_info(:timeout, %{idle_hibernate: idle_hibernate} = s) do + maybe_hibernate(idle_hibernate, {:noreply, s}) end def handle_info(msg, %{mod: mod} = s) do Logger.info(fn() -> @@ -568,12 +569,12 @@ defmodule DBConnection.Connection do end defp handle_timeout(%{client: nil, idle_timeout: idle_timeout, - hibernate?: hibernate?} = s) do + idle_hibernate: idle_hibernate} = s) do Process.send_after(self(), :timeout, idle_timeout) - maybe_hibernate(hibernate?, {:noreply, s}) + maybe_hibernate(idle_hibernate, {:noreply, s}) end - defp handle_timeout(%{hibernate?: hibernate?} = s) do - maybe_hibernate(hibernate?, {:noreply, s}) + defp handle_timeout(%{idle_hibernate: idle_hibernate} = s) do + maybe_hibernate(idle_hibernate, {:noreply, s}) end defp maybe_hibernate(true, res), do: Tuple.append(res, :hibernate) From e65800386068816845e3dab925dced80681c06a8 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Mon, 20 Nov 2017 16:34:04 +0800 Subject: [PATCH 3/9] add test case for idle hibernate --- integration_test/cases/hibernate_test.exs | 46 ++++++++++++++++++++++ integration_test/ownership/test_helper.exs | 2 +- integration_test/tests.exs | 1 + 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 integration_test/cases/hibernate_test.exs diff --git a/integration_test/cases/hibernate_test.exs b/integration_test/cases/hibernate_test.exs new file mode 100644 index 0000000..5934923 --- /dev/null +++ b/integration_test/cases/hibernate_test.exs @@ -0,0 +1,46 @@ +defmodule TestIdleHibernate do + use ExUnit.Case, async: true + + alias TestPool, as: P + alias TestAgent, as: A + + @tag :idle_hibernate + test "test idle_hibernate" do + parent = self() + stack = [ + fn(opts) -> + send(opts[:parent], {:hi, self()}) + {:ok, :state} + end, + fn(_) -> + send(parent, {:pong, self()}) + :timer.sleep(10) + {:ok, :state} + end, + fn(_) -> + send(parent, {:pong, self()}) + assert_receive {:continue, ^parent} + {:ok, :state} + end, + fn(_) -> + send(parent, {:pong, self()}) + :timer.sleep(:infinity) + end] + {:ok, agent} = A.start_link(stack) + + opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] + {:ok, pool} = P.start_link(opts) + assert_receive {:hi, conn} + assert_receive {:pong, ^conn} + assert_receive {:pong, ^conn} + send(conn, {:continue, self()}) + P.run(pool, fn(_) -> :ok end) + assert_receive {:pong, ^conn} + + assert [ + connect: [_], + ping: [:state], + ping: [:state], + ping: [:state]] = A.record(agent) + end +end diff --git a/integration_test/ownership/test_helper.exs b/integration_test/ownership/test_helper.exs index 27a8926..9c613d9 100644 --- a/integration_test/ownership/test_helper.exs +++ b/integration_test/ownership/test_helper.exs @@ -1,6 +1,6 @@ ExUnit.start([capture_log: true, assert_receive_timeout: 500, exclude: [:idle_timeout, :pool_overflow, :dequeue_disconnected, - :queue_timeout_raise]]) + :queue_timeout_raise, :idle_hibernate]]) Code.require_file "../../test/test_support.exs", __DIR__ diff --git a/integration_test/tests.exs b/integration_test/tests.exs index 04b9e6d..8101f17 100644 --- a/integration_test/tests.exs +++ b/integration_test/tests.exs @@ -13,3 +13,4 @@ Code.require_file "cases/queue_test.exs", __DIR__ Code.require_file "cases/stream_test.exs", __DIR__ Code.require_file "cases/transaction_execute_test.exs", __DIR__ Code.require_file "cases/transaction_test.exs", __DIR__ +Code.require_file "cases/hibernate_test.exs", __DIR__ From dbb23685ccbad4b7f11dfc3256461658a07e5ae2 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Tue, 21 Nov 2017 09:06:41 +0800 Subject: [PATCH 4/9] update unit test via cr --- integration_test/cases/backoff_test.exs | 16 +++++++- integration_test/cases/hibernate_test.exs | 46 ---------------------- integration_test/cases/idle_test.exs | 18 ++++++++- integration_test/ownership/test_helper.exs | 2 +- integration_test/tests.exs | 1 - 5 files changed, 33 insertions(+), 50 deletions(-) delete mode 100644 integration_test/cases/hibernate_test.exs diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index 3532d2b..e514346 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -5,6 +5,18 @@ defmodule BackoffTest do alias TestAgent, as: A test "backoff after failed initial connection attempt" do + agent = spawn_agent_backoff_after_failed() + opts = [agent: agent, parent: self(), backoff_min: 10] + execute_test_backoff_after_failed(agent, opts) + end + + test "backoff after failed initial connection attempt with idle_hibernate" do + agent = spawn_agent_backoff_after_failed() + opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] + execute_test_backoff_after_failed(agent, opts) + end + + defp spawn_agent_backoff_after_failed() do err = RuntimeError.exception("oops") stack = [ fn(opts) -> @@ -16,8 +28,10 @@ defmodule BackoffTest do {:ok, :state} end] {:ok, agent} = A.start_link(stack) + agent + end - opts = [agent: agent, parent: self(), backoff_min: 10] + defp execute_test_backoff_after_failed(agent, opts) do {:ok, _} = P.start_link(opts) assert_receive {:error, conn} assert_receive {:hi, ^conn} diff --git a/integration_test/cases/hibernate_test.exs b/integration_test/cases/hibernate_test.exs deleted file mode 100644 index 5934923..0000000 --- a/integration_test/cases/hibernate_test.exs +++ /dev/null @@ -1,46 +0,0 @@ -defmodule TestIdleHibernate do - use ExUnit.Case, async: true - - alias TestPool, as: P - alias TestAgent, as: A - - @tag :idle_hibernate - test "test idle_hibernate" do - parent = self() - stack = [ - fn(opts) -> - send(opts[:parent], {:hi, self()}) - {:ok, :state} - end, - fn(_) -> - send(parent, {:pong, self()}) - :timer.sleep(10) - {:ok, :state} - end, - fn(_) -> - send(parent, {:pong, self()}) - assert_receive {:continue, ^parent} - {:ok, :state} - end, - fn(_) -> - send(parent, {:pong, self()}) - :timer.sleep(:infinity) - end] - {:ok, agent} = A.start_link(stack) - - opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] - {:ok, pool} = P.start_link(opts) - assert_receive {:hi, conn} - assert_receive {:pong, ^conn} - assert_receive {:pong, ^conn} - send(conn, {:continue, self()}) - P.run(pool, fn(_) -> :ok end) - assert_receive {:pong, ^conn} - - assert [ - connect: [_], - ping: [:state], - ping: [:state], - ping: [:state]] = A.record(agent) - end -end diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index 9687fc1..cfcb342 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -6,6 +6,19 @@ defmodule TestIdle do @tag :idle_timeout test "ping after idle timeout" do + agent = spawn_agent() + opts = [agent: agent, parent: self(), idle_timeout: 50] + execute_test_case(agent, opts) + end + + @tag :idle_timeout + test "ping after idle timeout using hibernate" do + agent = spawn_agent() + opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] + execute_test_case(agent, opts) + end + + defp spawn_agent() do parent = self() stack = [ fn(opts) -> @@ -27,8 +40,10 @@ defmodule TestIdle do :timer.sleep(:infinity) end] {:ok, agent} = A.start_link(stack) + agent + end - opts = [agent: agent, parent: self(), idle_timeout: 50] + defp execute_test_case(agent, opts) do {:ok, pool} = P.start_link(opts) assert_receive {:hi, conn} assert_receive {:pong, ^conn} @@ -43,4 +58,5 @@ defmodule TestIdle do ping: [:state], ping: [:state]] = A.record(agent) end + end diff --git a/integration_test/ownership/test_helper.exs b/integration_test/ownership/test_helper.exs index 9c613d9..27a8926 100644 --- a/integration_test/ownership/test_helper.exs +++ b/integration_test/ownership/test_helper.exs @@ -1,6 +1,6 @@ ExUnit.start([capture_log: true, assert_receive_timeout: 500, exclude: [:idle_timeout, :pool_overflow, :dequeue_disconnected, - :queue_timeout_raise, :idle_hibernate]]) + :queue_timeout_raise]]) Code.require_file "../../test/test_support.exs", __DIR__ diff --git a/integration_test/tests.exs b/integration_test/tests.exs index 8101f17..04b9e6d 100644 --- a/integration_test/tests.exs +++ b/integration_test/tests.exs @@ -13,4 +13,3 @@ Code.require_file "cases/queue_test.exs", __DIR__ Code.require_file "cases/stream_test.exs", __DIR__ Code.require_file "cases/transaction_execute_test.exs", __DIR__ Code.require_file "cases/transaction_test.exs", __DIR__ -Code.require_file "cases/hibernate_test.exs", __DIR__ From 650be16bae3c70d2cffa4f3ff1f9ee460609cb54 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Tue, 21 Nov 2017 09:46:44 +0800 Subject: [PATCH 5/9] add test for hibernate --- integration_test/cases/backoff_test.exs | 23 +++++++++++++++++++++-- integration_test/cases/idle_test.exs | 23 +++++++++++++++++++++-- lib/db_connection/connection.ex | 20 +++++++++++++++++++- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index e514346..15bac14 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -4,6 +4,8 @@ defmodule BackoffTest do alias TestPool, as: P alias TestAgent, as: A + @ets_table_name_for_test :ets_for_test_hibernate + test "backoff after failed initial connection attempt" do agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10] @@ -11,9 +13,10 @@ defmodule BackoffTest do end test "backoff after failed initial connection attempt with idle_hibernate" do + create_ets_for_test() agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] - execute_test_backoff_after_failed(agent, opts) + execute_test_backoff_after_failed(agent, opts, true) end defp spawn_agent_backoff_after_failed() do @@ -31,7 +34,7 @@ defmodule BackoffTest do agent end - defp execute_test_backoff_after_failed(agent, opts) do + defp execute_test_backoff_after_failed(agent, opts, test_hibernate? \\ false) do {:ok, _} = P.start_link(opts) assert_receive {:error, conn} assert_receive {:hi, ^conn} @@ -39,6 +42,22 @@ defmodule BackoffTest do assert [ connect: [opts2], connect: [opts2]] = A.record(agent) + + if test_hibernate? do + assert [{:hibernate_times, _}] = + :ets.lookup(@ets_table_name_for_test, :hibernate_times) + end + end + + defp create_ets_for_test() do + case :ets.info(@ets_table_name_for_test) do + :undefined -> + :ets.new(@ets_table_name_for_test, [:named_table, :public]) + :ok + _ -> + :ets.delete_all_objects(@ets_table_name_for_test) + :ok + end end test "backoff after disconnect and failed connection attempt" do diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index cfcb342..ba0e968 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -4,6 +4,8 @@ defmodule TestIdle do alias TestPool, as: P alias TestAgent, as: A + @ets_table_name_for_test :ets_for_test_hibernate + @tag :idle_timeout test "ping after idle timeout" do agent = spawn_agent() @@ -13,9 +15,10 @@ defmodule TestIdle do @tag :idle_timeout test "ping after idle timeout using hibernate" do + create_ets_for_test() agent = spawn_agent() opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] - execute_test_case(agent, opts) + execute_test_case(agent, opts, true) end defp spawn_agent() do @@ -43,7 +46,7 @@ defmodule TestIdle do agent end - defp execute_test_case(agent, opts) do + defp execute_test_case(agent, opts, test_hibernate? \\ false) do {:ok, pool} = P.start_link(opts) assert_receive {:hi, conn} assert_receive {:pong, ^conn} @@ -57,6 +60,22 @@ defmodule TestIdle do ping: [:state], ping: [:state], ping: [:state]] = A.record(agent) + + if test_hibernate? and :sojourn != Mix.env() do + assert [{:hibernate_times, _}] = + :ets.lookup(@ets_table_name_for_test, :hibernate_times) + end + end + + defp create_ets_for_test() do + case :ets.info(@ets_table_name_for_test) do + :undefined -> + :ets.new(@ets_table_name_for_test, [:named_table, :public]) + :ok + _ -> + :ets.delete_all_objects(@ets_table_name_for_test) + :ok + end end end diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index 5b73fb5..83b284a 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -23,6 +23,9 @@ defmodule DBConnection.Connection do @idle_timeout 1_000 @idle_hibernate false + ## for unit test + @ets_table_name_for_test :ets_for_test_hibernate + ## DBConnection.Pool API @doc false @@ -577,9 +580,24 @@ defmodule DBConnection.Connection do maybe_hibernate(idle_hibernate, {:noreply, s}) end - defp maybe_hibernate(true, res), do: Tuple.append(res, :hibernate) + defp maybe_hibernate(true, res) do + maybe_update_test_data() + Tuple.append(res, :hibernate) + end defp maybe_hibernate(_, res), do: res + if Mix.env() in [:test, :connection, :poolboy, :sojourn, :ownership] do + defp maybe_update_test_data() do + :ets.update_counter(@ets_table_name_for_test, :hibernate_times, 1, + {:hibernate_times, 0}) + :ok + end + else + defp maybe_update_test_data() do + :ok + end + end + defp demonitor({_, mon}) when is_reference(mon) do Process.demonitor(mon, [:flush]) end From 5b3c7cbd2333f15b8ca30bd06bcd9533820c2db0 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Tue, 21 Nov 2017 11:33:40 +0800 Subject: [PATCH 6/9] update unit test for hibernate --- integration_test/cases/backoff_test.exs | 24 +++++------------------- integration_test/cases/idle_test.exs | 24 +++++------------------- lib/db_connection/connection.ex | 20 +------------------- 3 files changed, 11 insertions(+), 57 deletions(-) diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index 15bac14..a83e3e2 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -4,8 +4,6 @@ defmodule BackoffTest do alias TestPool, as: P alias TestAgent, as: A - @ets_table_name_for_test :ets_for_test_hibernate - test "backoff after failed initial connection attempt" do agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10] @@ -13,7 +11,6 @@ defmodule BackoffTest do end test "backoff after failed initial connection attempt with idle_hibernate" do - create_ets_for_test() agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] execute_test_backoff_after_failed(agent, opts, true) @@ -39,25 +36,14 @@ defmodule BackoffTest do assert_receive {:error, conn} assert_receive {:hi, ^conn} + if test_hibernate? and Mix.env() != :sojourn do + assert {:current_function, {:erlang, :hibernate, 3}} == + Process.info(conn, :current_function) + end + assert [ connect: [opts2], connect: [opts2]] = A.record(agent) - - if test_hibernate? do - assert [{:hibernate_times, _}] = - :ets.lookup(@ets_table_name_for_test, :hibernate_times) - end - end - - defp create_ets_for_test() do - case :ets.info(@ets_table_name_for_test) do - :undefined -> - :ets.new(@ets_table_name_for_test, [:named_table, :public]) - :ok - _ -> - :ets.delete_all_objects(@ets_table_name_for_test) - :ok - end end test "backoff after disconnect and failed connection attempt" do diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index ba0e968..6646db5 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -4,8 +4,6 @@ defmodule TestIdle do alias TestPool, as: P alias TestAgent, as: A - @ets_table_name_for_test :ets_for_test_hibernate - @tag :idle_timeout test "ping after idle timeout" do agent = spawn_agent() @@ -15,7 +13,6 @@ defmodule TestIdle do @tag :idle_timeout test "ping after idle timeout using hibernate" do - create_ets_for_test() agent = spawn_agent() opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] execute_test_case(agent, opts, true) @@ -49,8 +46,13 @@ defmodule TestIdle do defp execute_test_case(agent, opts, test_hibernate? \\ false) do {:ok, pool} = P.start_link(opts) assert_receive {:hi, conn} + if test_hibernate? and :sojourn != Mix.env() do + assert {:current_function, {:erlang, :hibernate, 3}} == + Process.info(conn, :current_function) + end assert_receive {:pong, ^conn} assert_receive {:pong, ^conn} + send(conn, {:continue, self()}) P.run(pool, fn(_) -> :ok end) assert_receive {:pong, ^conn} @@ -60,22 +62,6 @@ defmodule TestIdle do ping: [:state], ping: [:state], ping: [:state]] = A.record(agent) - - if test_hibernate? and :sojourn != Mix.env() do - assert [{:hibernate_times, _}] = - :ets.lookup(@ets_table_name_for_test, :hibernate_times) - end - end - - defp create_ets_for_test() do - case :ets.info(@ets_table_name_for_test) do - :undefined -> - :ets.new(@ets_table_name_for_test, [:named_table, :public]) - :ok - _ -> - :ets.delete_all_objects(@ets_table_name_for_test) - :ok - end end end diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index 83b284a..5b73fb5 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -23,9 +23,6 @@ defmodule DBConnection.Connection do @idle_timeout 1_000 @idle_hibernate false - ## for unit test - @ets_table_name_for_test :ets_for_test_hibernate - ## DBConnection.Pool API @doc false @@ -580,24 +577,9 @@ defmodule DBConnection.Connection do maybe_hibernate(idle_hibernate, {:noreply, s}) end - defp maybe_hibernate(true, res) do - maybe_update_test_data() - Tuple.append(res, :hibernate) - end + defp maybe_hibernate(true, res), do: Tuple.append(res, :hibernate) defp maybe_hibernate(_, res), do: res - if Mix.env() in [:test, :connection, :poolboy, :sojourn, :ownership] do - defp maybe_update_test_data() do - :ets.update_counter(@ets_table_name_for_test, :hibernate_times, 1, - {:hibernate_times, 0}) - :ok - end - else - defp maybe_update_test_data() do - :ok - end - end - defp demonitor({_, mon}) when is_reference(mon) do Process.demonitor(mon, [:flush]) end From e1590494cd81e376261aa7d9592a709c13028e85 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Wed, 22 Nov 2017 00:17:09 +0800 Subject: [PATCH 7/9] use tag rather than Mix.env and add hibernate timer --- integration_test/cases/backoff_test.exs | 3 ++- integration_test/cases/idle_test.exs | 4 ++-- integration_test/ownership/test_helper.exs | 2 +- integration_test/sojourn/test_helper.exs | 3 ++- lib/db_connection/connection.ex | 28 ++++++++++++++++++---- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index a83e3e2..fdd996c 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -10,6 +10,7 @@ defmodule BackoffTest do execute_test_backoff_after_failed(agent, opts) end + @tag :idle_hibernate_backoff test "backoff after failed initial connection attempt with idle_hibernate" do agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] @@ -36,7 +37,7 @@ defmodule BackoffTest do assert_receive {:error, conn} assert_receive {:hi, ^conn} - if test_hibernate? and Mix.env() != :sojourn do + if test_hibernate? do assert {:current_function, {:erlang, :hibernate, 3}} == Process.info(conn, :current_function) end diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index 6646db5..9a2cb63 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -11,7 +11,7 @@ defmodule TestIdle do execute_test_case(agent, opts) end - @tag :idle_timeout + @tag :idle_hibernate test "ping after idle timeout using hibernate" do agent = spawn_agent() opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] @@ -46,7 +46,7 @@ defmodule TestIdle do defp execute_test_case(agent, opts, test_hibernate? \\ false) do {:ok, pool} = P.start_link(opts) assert_receive {:hi, conn} - if test_hibernate? and :sojourn != Mix.env() do + if test_hibernate? do assert {:current_function, {:erlang, :hibernate, 3}} == Process.info(conn, :current_function) end diff --git a/integration_test/ownership/test_helper.exs b/integration_test/ownership/test_helper.exs index 27a8926..9c613d9 100644 --- a/integration_test/ownership/test_helper.exs +++ b/integration_test/ownership/test_helper.exs @@ -1,6 +1,6 @@ ExUnit.start([capture_log: true, assert_receive_timeout: 500, exclude: [:idle_timeout, :pool_overflow, :dequeue_disconnected, - :queue_timeout_raise]]) + :queue_timeout_raise, :idle_hibernate]]) Code.require_file "../../test/test_support.exs", __DIR__ diff --git a/integration_test/sojourn/test_helper.exs b/integration_test/sojourn/test_helper.exs index 33eb960..00433aa 100644 --- a/integration_test/sojourn/test_helper.exs +++ b/integration_test/sojourn/test_helper.exs @@ -10,7 +10,8 @@ case :erlang.system_info(:otp_release) do ExUnit.start([exclude: [:test]]) _ -> ExUnit.start([capture_log: :true, assert_receive_timeout: 500, - exclude: [:enqueue_disconnected, :queue_timeout_exit]]) + exclude: [:enqueue_disconnected, :queue_timeout_exit, + :idle_hibernate_backoff, :idle_hibernate]]) {:ok, _} = TestPool.ensure_all_started() end diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index 5b73fb5..fd61440 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -109,6 +109,7 @@ defmodule DBConnection.Connection do s = %{mod: mod, opts: opts, state: nil, client: :closed, broker: broker, regulator: regulator, lock: nil, queue: queue, timer: nil, + hibernate_timer: nil, backoff: Backoff.new(opts), after_connect: Keyword.get(opts, :after_connect), after_connect_timeout: Keyword.get(opts, :after_connect_timeout, @@ -355,6 +356,11 @@ defmodule DBConnection.Connection do end end + def handle_info({:timeout, timer, :trigger_hibernate}, + %{idle_hibernate: idle_hibernate} = s) when is_reference(timer) do + maybe_hibernate(idle_hibernate, {:noreply, %{s | hibernate_timer: nil}}) + end + def handle_info(:timeout, %{client: nil, broker: nil} = s) do %{mod: mod, state: state} = s case apply(mod, :ping, [state]) do @@ -380,7 +386,7 @@ defmodule DBConnection.Connection do do_handle_info(msg, s) end def handle_info(:timeout, %{idle_hibernate: idle_hibernate} = s) do - maybe_hibernate(idle_hibernate, {:noreply, s}) + maybe_hibernate(idle_hibernate, {:noreply, %{s | hibernate_timer: nil}}) end def handle_info(msg, %{mod: mod} = s) do Logger.info(fn() -> @@ -569,9 +575,15 @@ defmodule DBConnection.Connection do end defp handle_timeout(%{client: nil, idle_timeout: idle_timeout, - idle_hibernate: idle_hibernate} = s) do - Process.send_after(self(), :timeout, idle_timeout) - maybe_hibernate(idle_hibernate, {:noreply, s}) + idle_hibernate: true, + hibernate_timer: hibernate_timer} = s) do + cancel_timer(hibernate_timer) + hibernate_timer = Process.send_after(self(), :timeout, idle_timeout) + # hibernate_timer = start_timer(self(), idle_timeout, :trigger_hibernate) + maybe_hibernate(true, {:noreply, %{s | hibernate_timer: hibernate_timer}}) + end + defp handle_timeout(%{client: nil, idle_timeout: idle_timeout} = s) do + {:noreply, s, idle_timeout} end defp handle_timeout(%{idle_hibernate: idle_hibernate} = s) do maybe_hibernate(idle_hibernate, {:noreply, s}) @@ -593,11 +605,15 @@ defmodule DBConnection.Connection do defp start_timer(pid, timeout) do :erlang.start_timer(timeout, self(), {__MODULE__, pid, timeout}) end + defp start_timer(_, :infinity, _), do: nil + defp start_timer(pid, timeout, msg) do + :erlang.start_timer(timeout, pid, msg) + end defp cancel_timer(nil), do: :ok defp cancel_timer(timer) do case :erlang.cancel_timer(timer) do - false -> flush_timer(timer) + false -> :erlang.read_timer(timer) and flush_timer(timer) _ -> :ok end end @@ -606,6 +622,8 @@ defmodule DBConnection.Connection do receive do {:timeout, ^timer, {__MODULE__, _, _}} -> :ok + {:timeout, ^timer, :trigger_hibernate} -> + :ok after 0 -> raise ArgumentError, "timer #{inspect(timer)} does not exist" From 95d0a580f8b3786e2e57a9387230982b80cd4228 Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Wed, 22 Nov 2017 01:55:06 +0800 Subject: [PATCH 8/9] modified via cr --- integration_test/cases/backoff_test.exs | 1 - integration_test/cases/idle_test.exs | 2 +- integration_test/ownership/test_helper.exs | 2 +- integration_test/sojourn/test_helper.exs | 3 +- lib/db_connection/connection.ex | 42 ++++++++++------------ 5 files changed, 22 insertions(+), 28 deletions(-) diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index fdd996c..a15ce5d 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -10,7 +10,6 @@ defmodule BackoffTest do execute_test_backoff_after_failed(agent, opts) end - @tag :idle_hibernate_backoff test "backoff after failed initial connection attempt with idle_hibernate" do agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index 9a2cb63..0f5b75a 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -11,7 +11,7 @@ defmodule TestIdle do execute_test_case(agent, opts) end - @tag :idle_hibernate + @tag :idle_timeout test "ping after idle timeout using hibernate" do agent = spawn_agent() opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] diff --git a/integration_test/ownership/test_helper.exs b/integration_test/ownership/test_helper.exs index 9c613d9..27a8926 100644 --- a/integration_test/ownership/test_helper.exs +++ b/integration_test/ownership/test_helper.exs @@ -1,6 +1,6 @@ ExUnit.start([capture_log: true, assert_receive_timeout: 500, exclude: [:idle_timeout, :pool_overflow, :dequeue_disconnected, - :queue_timeout_raise, :idle_hibernate]]) + :queue_timeout_raise]]) Code.require_file "../../test/test_support.exs", __DIR__ diff --git a/integration_test/sojourn/test_helper.exs b/integration_test/sojourn/test_helper.exs index 00433aa..33eb960 100644 --- a/integration_test/sojourn/test_helper.exs +++ b/integration_test/sojourn/test_helper.exs @@ -10,8 +10,7 @@ case :erlang.system_info(:otp_release) do ExUnit.start([exclude: [:test]]) _ -> ExUnit.start([capture_log: :true, assert_receive_timeout: 500, - exclude: [:enqueue_disconnected, :queue_timeout_exit, - :idle_hibernate_backoff, :idle_hibernate]]) + exclude: [:enqueue_disconnected, :queue_timeout_exit]]) {:ok, _} = TestPool.ensure_all_started() end diff --git a/lib/db_connection/connection.ex b/lib/db_connection/connection.ex index fd61440..9e71bcd 100644 --- a/lib/db_connection/connection.ex +++ b/lib/db_connection/connection.ex @@ -109,7 +109,7 @@ defmodule DBConnection.Connection do s = %{mod: mod, opts: opts, state: nil, client: :closed, broker: broker, regulator: regulator, lock: nil, queue: queue, timer: nil, - hibernate_timer: nil, + idle_timer: nil, backoff: Backoff.new(opts), after_connect: Keyword.get(opts, :after_connect), after_connect_timeout: Keyword.get(opts, :after_connect_timeout, @@ -338,6 +338,17 @@ defmodule DBConnection.Connection do end end + def handle_info({:timeout, timer, {__MODULE__, :idle, _}}, s) + when is_reference(timer) do + %{mod: mod, state: state} = s + case apply(mod, :ping, [state]) do + {:ok, state} -> + handle_timeout(%{s | state: state, idle_timer: nil}) + {:disconnect, err, state} -> + {:disconnect, {:log, err}, %{s | state: state, idle_timer: nil}} + end + end + def handle_info({:timeout, timer, {__MODULE__, pid, timeout}}, %{timer: timer} = s) when is_reference(timer) do message = "client #{inspect pid} timed out because " <> @@ -356,11 +367,6 @@ defmodule DBConnection.Connection do end end - def handle_info({:timeout, timer, :trigger_hibernate}, - %{idle_hibernate: idle_hibernate} = s) when is_reference(timer) do - maybe_hibernate(idle_hibernate, {:noreply, %{s | hibernate_timer: nil}}) - end - def handle_info(:timeout, %{client: nil, broker: nil} = s) do %{mod: mod, state: state} = s case apply(mod, :ping, [state]) do @@ -385,9 +391,6 @@ defmodule DBConnection.Connection do def handle_info(msg, %{client: {_, :connect}} = s) do do_handle_info(msg, s) end - def handle_info(:timeout, %{idle_hibernate: idle_hibernate} = s) do - maybe_hibernate(idle_hibernate, {:noreply, %{s | hibernate_timer: nil}}) - end def handle_info(msg, %{mod: mod} = s) do Logger.info(fn() -> [inspect(mod), ?\s, ?(, inspect(self()), ") missed message: " | @@ -549,10 +552,10 @@ defmodule DBConnection.Connection do {:noreply, s} end - defp continue_ping(%{mod: mod, state: state} = s) do + defp continue_ping(%{mod: mod, state: state, idle_hibernate: idle_hibernate} = s) do case apply(mod, :ping, [state]) do {:ok, state} -> - continue_ask(%{s | idle_time: 0, state: state}) + maybe_hibernate(idle_hibernate, continue_ask(%{s | idle_time: 0, state: state})) {:disconnect, err, state} -> {:disconnect, {:log, err}, %{s | idle_time: 0, state: state}} end @@ -576,11 +579,10 @@ defmodule DBConnection.Connection do defp handle_timeout(%{client: nil, idle_timeout: idle_timeout, idle_hibernate: true, - hibernate_timer: hibernate_timer} = s) do - cancel_timer(hibernate_timer) - hibernate_timer = Process.send_after(self(), :timeout, idle_timeout) - # hibernate_timer = start_timer(self(), idle_timeout, :trigger_hibernate) - maybe_hibernate(true, {:noreply, %{s | hibernate_timer: hibernate_timer}}) + idle_timer: idle_timer} = s) do + cancel_timer(idle_timer) + idle_timer = start_timer(:idle, idle_timeout) + maybe_hibernate(true, {:noreply, %{s | idle_timer: idle_timer}}) end defp handle_timeout(%{client: nil, idle_timeout: idle_timeout} = s) do {:noreply, s, idle_timeout} @@ -605,15 +607,11 @@ defmodule DBConnection.Connection do defp start_timer(pid, timeout) do :erlang.start_timer(timeout, self(), {__MODULE__, pid, timeout}) end - defp start_timer(_, :infinity, _), do: nil - defp start_timer(pid, timeout, msg) do - :erlang.start_timer(timeout, pid, msg) - end defp cancel_timer(nil), do: :ok defp cancel_timer(timer) do case :erlang.cancel_timer(timer) do - false -> :erlang.read_timer(timer) and flush_timer(timer) + false -> flush_timer(timer) _ -> :ok end end @@ -622,8 +620,6 @@ defmodule DBConnection.Connection do receive do {:timeout, ^timer, {__MODULE__, _, _}} -> :ok - {:timeout, ^timer, :trigger_hibernate} -> - :ok after 0 -> raise ArgumentError, "timer #{inspect(timer)} does not exist" From 3144264e7d9e4aafc0081f06bdf355dd08703fcf Mon Sep 17 00:00:00 2001 From: "taotao.lin" Date: Wed, 22 Nov 2017 08:25:10 +0800 Subject: [PATCH 9/9] disable idle hibernate test when sojourn --- integration_test/cases/backoff_test.exs | 1 + integration_test/cases/idle_test.exs | 1 + integration_test/sojourn/test_helper.exs | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/integration_test/cases/backoff_test.exs b/integration_test/cases/backoff_test.exs index a15ce5d..0913601 100644 --- a/integration_test/cases/backoff_test.exs +++ b/integration_test/cases/backoff_test.exs @@ -10,6 +10,7 @@ defmodule BackoffTest do execute_test_backoff_after_failed(agent, opts) end + @tag :idle_hibernate test "backoff after failed initial connection attempt with idle_hibernate" do agent = spawn_agent_backoff_after_failed() opts = [agent: agent, parent: self(), backoff_min: 10, idle_hibernate: true] diff --git a/integration_test/cases/idle_test.exs b/integration_test/cases/idle_test.exs index 0f5b75a..d9c2ff5 100644 --- a/integration_test/cases/idle_test.exs +++ b/integration_test/cases/idle_test.exs @@ -12,6 +12,7 @@ defmodule TestIdle do end @tag :idle_timeout + @tag :idle_hibernate test "ping after idle timeout using hibernate" do agent = spawn_agent() opts = [agent: agent, parent: self(), idle_timeout: 50, idle_hibernate: true] diff --git a/integration_test/sojourn/test_helper.exs b/integration_test/sojourn/test_helper.exs index 33eb960..64db9a6 100644 --- a/integration_test/sojourn/test_helper.exs +++ b/integration_test/sojourn/test_helper.exs @@ -10,7 +10,7 @@ case :erlang.system_info(:otp_release) do ExUnit.start([exclude: [:test]]) _ -> ExUnit.start([capture_log: :true, assert_receive_timeout: 500, - exclude: [:enqueue_disconnected, :queue_timeout_exit]]) + exclude: [:enqueue_disconnected, :queue_timeout_exit, :idle_hibernate]]) {:ok, _} = TestPool.ensure_all_started() end