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

chore(electric): Increase client ping interval from 5 to 20 seconds #1334

Merged
merged 3 commits into from
Jun 6, 2024
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
5 changes: 5 additions & 0 deletions .changeset/poor-deers-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@core/electric": patch
---

Increase client ping interval from 5 to 20 seconds.
4 changes: 3 additions & 1 deletion components/electric/lib/electric/satellite/protocol/state.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule Electric.Satellite.Protocol.State do

defstruct auth_passed: false,
auth: nil,
last_ping_time: nil,
last_msg_time: nil,
client_id: nil,
expiration_timer: nil,
Expand All @@ -23,7 +24,8 @@ defmodule Electric.Satellite.Protocol.State do
@type t() :: %__MODULE__{
auth_passed: boolean(),
auth: nil | Electric.Satellite.Auth.t(),
last_msg_time: :erlang.timestamp() | nil | :ping_sent,
last_ping_time: :erlang.timestamp() | nil,
last_msg_time: :erlang.timestamp() | nil | :missed_one_ping_interval,
client_id: String.t() | nil,
expiration_timer: {reference(), reference()} | nil,
in_rep: InRep.t(),
Expand Down
35 changes: 21 additions & 14 deletions components/electric/lib/electric/satellite/ws_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ defmodule Electric.Satellite.WebsocketServer do
alias Electric.Satellite.Protocol.InRep
alias Electric.Satellite.Protocol.Telemetry

# in milliseconds
@ping_interval 5_000
# Time interval at which the server will ping the client, in milliseconds
# This will also be the time that the server will wait for a response to the ping
# before disconnecting the client
@ping_interval 20_000

def reg_name(name) do
Electric.name(__MODULE__, name)
Expand Down Expand Up @@ -172,18 +174,23 @@ defmodule Electric.Satellite.WebsocketServer do
handle_producer_msg({out_rep.pid, out_rep.stage_sub}, {:cancel, :down}, state)
end

def handle_info(
{:timeout, :ping_timer},
%State{last_msg_time: :missed_one_ping_interval} = state
) do
Logger.warning("Client is not responding to ping, disconnecting")
{:stop, :normal, {1005, "Client not responding to pings"}, state}
end

def handle_info({:timeout, :ping_timer}, %State{} = state) do
case state.last_msg_time do
:ping_sent ->
Logger.info("Client is not responding to ping, disconnecting")
{:stop, :normal, {1005, "Client not responding to pings"}, state}

last_msg_time ->
if :timer.now_diff(:erlang.timestamp(), last_msg_time) > @ping_interval * 1000 do
{:push, {:ping, ""}, schedule_ping(%{state | last_msg_time: :ping_sent})}
else
{:ok, schedule_ping(state)}
end
timediff = :timer.now_diff(state.last_msg_time, state.last_ping_time)

if timediff > 0 do
# Got a "pong" or a regular message from the client, scheduling the next ping.
{:ok, schedule_ping(state)}
else
# Haven't received anything from the client within one ping interval.
{:push, {:ping, ""}, schedule_ping(%{state | last_msg_time: :missed_one_ping_interval})}
end
end

Expand Down Expand Up @@ -429,7 +436,7 @@ defmodule Electric.Satellite.WebsocketServer do
@spec schedule_ping(State.t()) :: State.t()
defp schedule_ping(%State{} = state) do
Process.send_after(self(), {:timeout, :ping_timer}, @ping_interval)
state
%{state | last_ping_time: :erlang.timestamp()}
end

if Mix.env() == :test do
Expand Down
Loading