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

…1334)

Supersedes #1228.

---------

Co-authored-by: David Martos <davidmartos96@gmail.com>

.changeset/poor-deers-marry.md

@@ -0,0 +1,5 @@
+---
+"@core/electric": patch
+---
+
+Increase client ping interval from 5 to 20 seconds.

components/electric/lib/electric/satellite/protocol/state.ex

@@ -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,
@@ -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(),

components/electric/lib/electric/satellite/ws_server.ex

@@ -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)
@@ -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
 
@@ -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