Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion integration_test/cases/backoff_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ 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

@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]
execute_test_backoff_after_failed(agent, opts, true)
end

defp spawn_agent_backoff_after_failed() do
err = RuntimeError.exception("oops")
stack = [
fn(opts) ->
Expand All @@ -16,12 +29,19 @@ 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, test_hibernate? \\ false) do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If theres enough logic to share a function we could try to simplify the test when hibenating and not share function. We might have overlapping tests.

{:ok, _} = P.start_link(opts)
assert_receive {:error, conn}
assert_receive {:hi, ^conn}

if test_hibernate? do
assert {:current_function, {:erlang, :hibernate, 3}} ==
Process.info(conn, :current_function)
end

assert [
connect: [opts2],
connect: [opts2]] = A.record(agent)
Expand Down
24 changes: 23 additions & 1 deletion integration_test/cases/idle_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ 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
@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]
execute_test_case(agent, opts, true)
end

defp spawn_agent() do
parent = self()
stack = [
fn(opts) ->
Expand All @@ -27,12 +41,19 @@ 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, test_hibernate? \\ false) do
{:ok, pool} = P.start_link(opts)
assert_receive {:hi, conn}
if test_hibernate? 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}
Expand All @@ -43,4 +64,5 @@ defmodule TestIdle do
ping: [:state],
ping: [:state]] = A.record(agent)
end

end
2 changes: 1 addition & 1 deletion integration_test/sojourn/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
44 changes: 35 additions & 9 deletions lib/db_connection/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -108,12 +109,14 @@ defmodule DBConnection.Connection do

s = %{mod: mod, opts: opts, state: nil, client: :closed, broker: broker,
regulator: regulator, lock: nil, queue: queue, 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,
@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,
idle_hibernate: Keyword.get(opts, :idle_hibernate, @idle_hibernate)}
if mode == :connection and Keyword.get(opts, :sync_connect, false) do
connect(:init, s)
else
Expand All @@ -130,7 +133,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, idle_hibernate: idle_hibernate} = s
case apply(mod, :connect, [opts]) do
{:ok, state} when after_connect != nil ->
ref = make_ref()
Expand All @@ -153,7 +156,7 @@ defmodule DBConnection.Connection do
end)
done_lock(regulator, lock)
{timeout, backoff} = Backoff.backoff(backoff)
{:backoff, timeout, %{s | lock: nil, backoff: backoff}}
maybe_hibernate(idle_hibernate, {:backoff, timeout, %{s | lock: nil, backoff: backoff}})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test this part with the other backoff handling

end
end

Expand Down Expand Up @@ -335,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 " <>
Expand Down Expand Up @@ -538,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
Expand All @@ -563,10 +577,22 @@ defmodule DBConnection.Connection do
end
end

defp handle_timeout(%{client: nil, idle_timeout: idle_timeout,
idle_hibernate: true,
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}
end
defp handle_timeout(s), do: {: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)
defp maybe_hibernate(_, res), do: res

defp demonitor({_, mon}) when is_reference(mon) do
Process.demonitor(mon, [:flush])
Expand Down