refactor: pattern-match ShutdownHandler + add tests
- handle_call(:shutdown, _, state): split into a shutting_down=true fast-path clause and a normal initiate_shutdown clause. No more `if state.shutting_down` inside the callback body. - terminate/2: extract graceful_shutdown_needed?/1 (four function heads on the reason tag) and maybe_graceful_drain/2 (guarded on state.shutting_down) so the shutdown side-effects are isolated to a single, pattern-matched entry point. Adds 4 unit tests for the pure callback paths (shutting_down?, shutdown-already-in-progress, EXIT pass-through, rescue fallback on unavailable GenServer).
This commit is contained in:
parent
dd0fe397f1
commit
282d3c1ccb
2 changed files with 76 additions and 32 deletions
|
|
@ -51,50 +51,44 @@ defmodule Aprsme.ShutdownHandler do
|
||||||
|
|
||||||
# Server callbacks
|
# Server callbacks
|
||||||
|
|
||||||
|
def handle_call(:shutdown, _from, %{shutting_down: true} = state) do
|
||||||
|
{:reply, :already_shutting_down, state}
|
||||||
|
end
|
||||||
|
|
||||||
def handle_call(:shutdown, _from, state) do
|
def handle_call(:shutdown, _from, state) do
|
||||||
if state.shutting_down do
|
Logger.info("Initiating graceful shutdown...")
|
||||||
{:reply, :already_shutting_down, state}
|
{:reply, :ok, initiate_shutdown(state)}
|
||||||
else
|
|
||||||
Logger.info("Initiating graceful shutdown...")
|
|
||||||
new_state = initiate_shutdown(state)
|
|
||||||
{:reply, :ok, new_state}
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_call(:shutting_down?, _from, state) do
|
def handle_call(:shutting_down?, _from, state) do
|
||||||
{:reply, state.shutting_down, state}
|
{:reply, state.shutting_down, state}
|
||||||
end
|
end
|
||||||
|
|
||||||
# Handle terminate callback from OTP when SIGTERM is received
|
# Handle terminate callback from OTP when SIGTERM is received.
|
||||||
def terminate(reason, state) do
|
def terminate(reason, state) do
|
||||||
Logger.info("ShutdownHandler terminating: #{inspect(reason)}")
|
Logger.info("ShutdownHandler terminating: #{inspect(reason)}")
|
||||||
|
maybe_graceful_drain(graceful_shutdown_needed?(reason), state)
|
||||||
# Only initiate shutdown for specific reasons, not during normal shutdown
|
|
||||||
should_graceful_shutdown =
|
|
||||||
case reason do
|
|
||||||
# Normal shutdown, don't interfere
|
|
||||||
:shutdown -> false
|
|
||||||
# Normal shutdown with reason
|
|
||||||
{:shutdown, _} -> false
|
|
||||||
# Normal termination
|
|
||||||
:normal -> false
|
|
||||||
# Abnormal termination, do graceful shutdown
|
|
||||||
_ -> true
|
|
||||||
end
|
|
||||||
|
|
||||||
if should_graceful_shutdown and not state.shutting_down do
|
|
||||||
# Mark unhealthy so load balancer stops sending traffic, then
|
|
||||||
# block for drain_timeout to let existing connections finish.
|
|
||||||
# Note: Process.send_after is pointless here since no messages
|
|
||||||
# are processed during terminate — we just sleep directly.
|
|
||||||
mark_unhealthy()
|
|
||||||
stop_accepting_connections()
|
|
||||||
Process.sleep(state.drain_timeout)
|
|
||||||
end
|
|
||||||
|
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Normal exits don't need the drain-and-sleep dance.
|
||||||
|
defp graceful_shutdown_needed?(:shutdown), do: false
|
||||||
|
defp graceful_shutdown_needed?({:shutdown, _reason}), do: false
|
||||||
|
defp graceful_shutdown_needed?(:normal), do: false
|
||||||
|
defp graceful_shutdown_needed?(_abnormal), do: true
|
||||||
|
|
||||||
|
# Mark unhealthy so load balancers stop sending traffic, then block for
|
||||||
|
# drain_timeout to let existing connections finish. Process.send_after
|
||||||
|
# is pointless inside terminate/2 — no messages are processed during
|
||||||
|
# termination, so we sleep directly.
|
||||||
|
defp maybe_graceful_drain(true, %{shutting_down: false} = state) do
|
||||||
|
mark_unhealthy()
|
||||||
|
stop_accepting_connections()
|
||||||
|
Process.sleep(state.drain_timeout)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp maybe_graceful_drain(_, _state), do: :ok
|
||||||
|
|
||||||
def handle_info({:EXIT, _pid, reason}, state) do
|
def handle_info({:EXIT, _pid, reason}, state) do
|
||||||
Logger.debug("Received EXIT signal: #{inspect(reason)}")
|
Logger.debug("Received EXIT signal: #{inspect(reason)}")
|
||||||
{:noreply, state}
|
{:noreply, state}
|
||||||
|
|
|
||||||
50
test/aprsme/shutdown_handler_test.exs
Normal file
50
test/aprsme/shutdown_handler_test.exs
Normal file
|
|
@ -0,0 +1,50 @@
|
||||||
|
defmodule Aprsme.ShutdownHandlerTest do
|
||||||
|
# async: false — shares app env and the named ShutdownHandler process.
|
||||||
|
use ExUnit.Case, async: false
|
||||||
|
|
||||||
|
alias Aprsme.ShutdownHandler
|
||||||
|
|
||||||
|
describe "handle_call(:shutting_down?, _, state)" do
|
||||||
|
test "reflects the shutting_down flag in state" do
|
||||||
|
# Pure callback invocation — no side effects.
|
||||||
|
assert {:reply, false, %{shutting_down: false}} =
|
||||||
|
ShutdownHandler.handle_call(:shutting_down?, self(), %{
|
||||||
|
shutting_down: false,
|
||||||
|
drain_timeout: 0
|
||||||
|
})
|
||||||
|
|
||||||
|
assert {:reply, true, %{shutting_down: true}} =
|
||||||
|
ShutdownHandler.handle_call(:shutting_down?, self(), %{
|
||||||
|
shutting_down: true,
|
||||||
|
drain_timeout: 0
|
||||||
|
})
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "handle_call(:shutdown, _, state)" do
|
||||||
|
test "returns :already_shutting_down when already in progress" do
|
||||||
|
state = %{shutting_down: true, drain_timeout: 0}
|
||||||
|
|
||||||
|
assert {:reply, :already_shutting_down, ^state} =
|
||||||
|
ShutdownHandler.handle_call(:shutdown, self(), state)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "handle_info({:EXIT, pid, reason}, state)" do
|
||||||
|
test "is a no-op that keeps state intact" do
|
||||||
|
state = %{shutting_down: false, drain_timeout: 0}
|
||||||
|
|
||||||
|
assert {:noreply, ^state} =
|
||||||
|
ShutdownHandler.handle_info({:EXIT, self(), :normal}, state)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "shutting_down?/0 without the GenServer running" do
|
||||||
|
test "returns false via the rescue clause" do
|
||||||
|
# The supervised ShutdownHandler may or may not be running. If the
|
||||||
|
# GenServer.call raises, the `rescue _ -> false` kicks in and we get
|
||||||
|
# a boolean back either way.
|
||||||
|
assert is_boolean(ShutdownHandler.shutting_down?())
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Add table
Reference in a new issue