Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backoff reset #71

Merged
merged 1 commit into from
Jul 19, 2023
Merged
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
30 changes: 17 additions & 13 deletions lib/mllp/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,9 @@ defmodule MLLP.Client do
end
end

def disconnected(:state_timeout, :reconnect, data) do
def disconnected(:state_timeout, :reconnect, _data) do
actions = [{:next_event, :internal, :connect}]
{:keep_state, data, actions}
{:keep_state_and_data, actions}
end

def disconnected({:call, from}, :reconnect, _data) do
Expand All @@ -425,8 +425,12 @@ defmodule MLLP.Client do
end

def disconnected({:call, from}, {:send, _message, _options}, data) do
actions = [{:reply, from, {:error, new_error(:send, data.tcp_error)}}]
{:keep_state_and_data, actions}
actions = [
{:reply, from, {:error, new_error(:send, data.tcp_error)}},
{:next_event, :internal, :connect}
]

{:keep_state, maybe_reset_reconnection_timeout(data), actions}
end

def disconnected({:call, from}, :is_connected, _data) do
Expand Down Expand Up @@ -733,13 +737,6 @@ defmodule MLLP.Client do
|> Map.put(:tcp_error, error)
end

defp backoff_succeed(%State{backoff: nil} = data), do: data

defp backoff_succeed(%State{backoff: backoff} = data) do
{_, new_backoff} = :backoff.succeed(backoff)
%{data | backoff: new_backoff}
end

defp attempt_connection(%State{} = data) do
telemetry(:status, %{status: :connecting}, data)
opts = fixed_socket_opts() ++ data.socket_opts ++ data.tls_opts
Expand All @@ -748,7 +745,7 @@ defmodule MLLP.Client do
{:ok, socket} ->
data1 =
data
|> backoff_succeed()
|> maybe_reset_reconnection_timeout()

telemetry(:status, %{status: :connected}, data1)
{:ok, %{data1 | socket: socket, tcp_error: nil}}
Expand Down Expand Up @@ -779,6 +776,13 @@ defmodule MLLP.Client do
%{data | backoff: new_backoff}
end

defp maybe_reset_reconnection_timeout(%State{backoff: nil} = data), do: data

defp maybe_reset_reconnection_timeout(%State{backoff: backoff} = data) do
{_, new_backoff} = :backoff.succeed(backoff)
%{data | backoff: new_backoff}
end

defp telemetry(_event_name, _measurements, %State{telemetry_module: nil} = _metadata) do
:ok
end
Expand Down Expand Up @@ -808,7 +812,7 @@ defmodule MLLP.Client do
end

def default_socket_opts() do
[send_timeout: 60_000, send_timeout_close: true]
[send_timeout: 60_000, send_timeout_close: true, keepalive: true]
end

def fixed_socket_opts() do
Expand Down
6 changes: 4 additions & 2 deletions test/client_and_receiver_integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,14 @@ defmodule ClientAndReceiverIntegrationTest do
)

capture_log(fn ->
{:error, _} = MLLP.Client.send(client_pid, "MSH|NOREPLY", %{reply_timeout: 1})
{:error, %MLLP.Client.Error{reason: :timeout}} =
MLLP.Client.send(client_pid, "MSH|NOREPLY", %{reply_timeout: 1})
end)

# Wait for reconnect timer
Process.sleep(10)
assert Process.alive?(client_pid)
assert MLLP.Client.is_connected?(client_pid)
{:ok, _} = MLLP.Client.send(client_pid, "MSH|REPLY")

assert Enum.count(open_ports_for_pid(client_pid)) == 1
end
Expand Down
29 changes: 28 additions & 1 deletion test/client_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,27 @@ defmodule ClientTest do

assert match?({:backoff, 1, 180, 2, :normal, _, _}, state.backoff)
end

test "the backoff will be reset every time 'send' call fails",
%{client: client, port: port} = _ctx do
# Stop the receiver...
MLLP.Receiver.stop(port)

# ... and wait for backoff to change; the delay below would set next backoff timeout to 4.
Process.sleep(3000 + 100)

{_fsm_state, state} = :sys.get_state(client)

{:backoff, 1, 180, backoff_timeout, :normal, _, _} = state.backoff
assert backoff_timeout == 4

{:error, _} = MLLP.Client.send(client, "no connection, this should fail")

{_fsm_state, state} = :sys.get_state(client)
{:backoff, 1, 180, backoff_timeout, :normal, _, _} = state.backoff

assert backoff_timeout == 2
end
end

describe "unexpected messages" do
Expand Down Expand Up @@ -182,11 +203,17 @@ defmodule ClientTest do
"this should fail as the connection will be dropped on unexpected packet"
)

refute Client.is_connected?(pid)
## Give it some time to reconnect
Process.sleep(10)
end)

assert String.contains?(log, Client.format_error(:unexpected_packet_received))
assert String.contains?(log, "Connection closed")
assert String.contains?(log, "Connection established")
## The connection had been closed, then re-established
{start_closed, _} = :binary.match(log, "Connection closed")
{start_established, _} = :binary.match(log, "Connection established")
assert start_closed < start_established
end
end

Expand Down