Skip to content

Commit

Permalink
Use monotonic_time for pruning stale Sonar nodes
Browse files Browse the repository at this point in the history
There's an apparent race condition in Sonar between pruning stale nodes
on `:ping` and updating the status after a notification. This primarily
happens in development for two reasons:

1. Development laptops are most prone to time warp because of system
   sleep
2. Apps only run a single node in development

Using `monotonic_time/1` instead of `system_time/1` guards against clock
drift/time warp effects.
  • Loading branch information
sorentwo committed Jun 25, 2024
1 parent b1d9c24 commit ad9b7ee
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
6 changes: 3 additions & 3 deletions lib/oban/sonar.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ defmodule Oban.Sonar do
end

def handle_info({:notification, :sonar, %{"node" => node} = payload}, state) do
time = Map.get(payload, "time", System.system_time(:millisecond))
time = Map.get(payload, "time", System.monotonic_time(:millisecond))

state =
state
Expand Down Expand Up @@ -111,8 +111,8 @@ defmodule Oban.Sonar do
end

defp prune_stale_nodes(state) do
stale = System.system_time(:millisecond) - state.interval * state.stale_mult
nodes = Map.reject(state.nodes, fn {_, recorded} -> recorded < stale end)
stale = System.monotonic_time(:millisecond) + state.interval * state.stale_mult
nodes = Map.reject(state.nodes, fn {_, recorded} -> recorded > stale end)

%{state | nodes: nodes}
end
Expand Down
6 changes: 3 additions & 3 deletions test/oban/sonar_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ defmodule Oban.SonarTest do

test "pruning stale nodes based on the last notification time" do
name = start_supervised_oban!(notifier: Oban.Notifiers.Isolated)
time = System.system_time(:millisecond)
time = System.monotonic_time(:millisecond)

Notifier.notify(name, :sonar, %{node: "web.1", time: time - :timer.seconds(29)})
Notifier.notify(name, :sonar, %{node: "web.2", time: time - :timer.seconds(31)})
Notifier.notify(name, :sonar, %{node: "web.1", time: time + :timer.seconds(29)})
Notifier.notify(name, :sonar, %{node: "web.2", time: time + :timer.seconds(31)})

nodes =
name
Expand Down

0 comments on commit ad9b7ee

Please sign in to comment.