From a763585a089432c691c9506e251257b06e1862d9 Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Tue, 6 Oct 2026 16:08:19 -0400 Subject: [PATCH 1/3] Avoid dumping unchanged DurableServer state during sync checks --- CHANGELOG.md | 1 + README.md | 12 +- lib/durable_server.ex | 43 +++++- test/durable_server_test.exs | 282 +++++++++++++++++++++++++++++++++++ 4 files changed, 329 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1e9a33..800b01d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased +- Skip dumping, serialization, and hashing during periodic and automatic sync when callback state is exactly unchanged and no earlier change is pending. Runtime state changes still use `dump_state/1` to determine whether a storage write is needed; explicit sync and lifecycle writes continue to evaluate the dump. - Resolve duplicate DurableServer ownership by a persisted, monotonically increasing lock epoch. Group now keeps the latest successful object-store claimant and retires only stale owners instead of killing every claimant and causing repeated anti-entropy restart loops. - Enforce cumulative sticky-placement gates during request-driven remote placement so an existing server cannot move to a fallback node before that level unlocks. - Allow an expired restart attempt to be reclaimed immediately by another node at an allowed sticky-placement level, while preserving strict placement when no level matches. diff --git a/README.md b/README.md index 789c27b..7d4cb6f 100644 --- a/README.md +++ b/README.md @@ -186,7 +186,7 @@ See `DurableServer.Backends.MirrorStore` for usage and an example rollout. DurableServer supports these options in the `init/1` return tuple: -- `:auto_sync` - Enable automatic periodic syncing (default: false) +- `:auto_sync` - Enable automatic syncing on callback return (default: false) - `:sync_every_ms` - Sync interval in milliseconds (default: 30_000) - `:meta` - Optional metadata included in the global registry @@ -205,6 +205,16 @@ State is synchronized to storage in these scenarios: eligible for a later sync. Storage conflicts remain fatal. 3. **Graceful shutdown**: State is always synced before termination +Periodic and automatic sync skip dumping, serialization, and hashing when callback state is +exactly unchanged and no earlier change is pending. Any runtime state change schedules a +check of `dump_state/1`; if its output is unchanged, the check skips the storage write and +clears the pending flag. Failed writes remain pending for a later sync. + +`dump_state/1` should deterministically derive its output from the supplied state. Changes +to external data, such as ETS or the process dictionary, cannot be detected by state tracking; +use an explicit sync to re-evaluate the dump when needed. Explicit sync and lifecycle writes +always evaluate `dump_state/1`. + Manual synchronization performs storage work inline. A `GenServer.call/3` timeout should be long enough to cover the configured backend's retry window. A caller timeout does not cancel storage work already running in the DurableServer process. diff --git a/lib/durable_server.ex b/lib/durable_server.ex index 18c44fd..59d221b 100644 --- a/lib/durable_server.ex +++ b/lib/durable_server.ex @@ -139,7 +139,7 @@ defmodule DurableServer do DurableServer supports these options in the `init/1` or `init/2` return tuple: - - `:auto_sync` - Enable automatic periodic syncing (default: false) + - `:auto_sync` - Enable automatic syncing on callback return (default: false) - `:sync_every_ms` - Sync interval in milliseconds (default: 30_000) - `:meta` - Optional metadata to include for the globally registered server which is returned alongside the pid with `DurableServer.Supervisor.lookup/2`. @@ -203,6 +203,11 @@ defmodule DurableServer do 3. **Graceful shutdown**: Automatically synced during normal termination, ie: cold deploys 4. **Before stopping**: When returning `{:stop, reason, state}` from callbacks + Periodic and automatic sync skip dumping and hashing while callback state is exactly + unchanged and no earlier change is pending. Runtime changes are checked with `dump_state/1` + before deciding whether a storage write is needed. Explicit sync and lifecycle writes + always evaluate the dump. + ## Stopping Behavior DurableServer supports different stop reasons with specific behaviors regarding exit signal propagation: @@ -757,6 +762,10 @@ defmodule DurableServer do - Transform the state shape for storage - Remove ephemeral data + Derive the output deterministically from the supplied state. Periodic and automatic + sync skip this callback when runtime state has not changed since the last successful + check. If the dump depends on external data, use explicit sync to re-evaluate it. + The returned value must be a plain map at the top level. Nested values are passed through to the configured backend as-is, so they only need to be encodable by the backend you are using. @@ -864,6 +873,7 @@ defmodule DurableServer do crash_history: [], module: nil, last_synced_user_state_hash: nil, + needs_sync_check: true, final_status_set: nil, terminator_handled: false, delete_requester: nil, @@ -2111,7 +2121,12 @@ defmodule DurableServer do end def handle_info({@durable, :sync}, %__MODULE__{} = state) do - case sync_to_storage(state) do + state = maybe_migrate_on_callback(state) + + sync_result = + if state.needs_sync_check, do: sync_to_storage(state), else: {:ok, state} + + case sync_result do {:ok, %DurableServer{} = new_state} -> {:noreply, schedule_sync(new_state)} @@ -2814,7 +2829,8 @@ defmodule DurableServer do def code_change(old_vsn, %__MODULE__{} = state, extra) do case state.module.code_change(old_vsn, state.user_state, extra) do {:ok, new_user_state} -> - {:ok, %{state | user_state: new_user_state}} + # The dumped shape can change across upgrades even if user state is unchanged. + {:ok, %{state | user_state: new_user_state, needs_sync_check: true}} {:error, reason} -> {:error, reason} @@ -2966,7 +2982,13 @@ defmodule DurableServer do {:ok, new_user_state} = state.module.code_change(state.vsn, state.user_state, current_vsn) - %{state | vsn: current_vsn, old_vsn: state.vsn, user_state: new_user_state} + %{ + state + | vsn: current_vsn, + old_vsn: state.vsn, + user_state: new_user_state, + needs_sync_check: true + } end end @@ -3340,11 +3362,16 @@ defmodule DurableServer do defp update_state(%__MODULE__{} = state, new_user_state) do new_user_state = validate_user_state!(new_user_state) - %{state | user_state: new_user_state} + + %{ + state + | user_state: new_user_state, + needs_sync_check: state.needs_sync_check or new_user_state !== state.user_state + } end defp auto_sync_to_storage(%DurableServer{module: module, key: key} = state, old_user_state) do - if state.auto_sync do + if state.auto_sync && state.needs_sync_check do old_user_state = old_user_state || state.user_state # if auto sync fails we continue, but log @@ -3415,14 +3442,14 @@ defmodule DurableServer do case put_object(new_state, storage_key(new_state), data) do {:ok, %DurableServer{} = new_state} -> - synced_state = %{new_state | status: new_meta.status} + synced_state = %{new_state | status: new_meta.status, needs_sync_check: false} maybe_apply_handle_sync(synced_state, old_user_state, user_state_changed?) {:error, reason} -> {:error, reason} end else - {:ok, new_state} + {:ok, %{new_state | needs_sync_check: false}} end end diff --git a/test/durable_server_test.exs b/test/durable_server_test.exs index 07ecb0c..8c709c3 100644 --- a/test/durable_server_test.exs +++ b/test/durable_server_test.exs @@ -187,6 +187,41 @@ defmodule DurableServerTest do {:shutdown, {:durable, {:fatal_exit, {:sync_failed, reason}}}} end + defp start_dirty_tracking_server(auto_sync \\ false) do + test_pid = self() + + after_put = fn _state, key, _data, _opts, result -> + send(test_pid, {:dirty_tracking_put, key, result}) + end + + {supervisor_name, _supervisor_pid, prefix} = + start_test_supervisor( + backend: {DurableServerTest.ConsistencyProbeBackend, after_put: after_put}, + init_info: %{probe_pid: test_pid, auto_sync: auto_sync} + ) + + key = "dirty-tracking-#{DurableServer.UUID.uuid4()}" + + {:ok, {pid, _meta}} = + DurableServer.Supervisor.start_child( + supervisor_name, + {DurableServerTest.DirtyTrackingServer, key: key, initial_state: %{count: 0}} + ) + + %{storage_backend: backend} = DurableServer.Supervisor.__get_config__(supervisor_name) + storage_key = prefix <> key + + assert_receive {:dirty_tracking_dump, ^pid, 0} + assert_receive {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + %{pid: pid, backend: backend, storage_key: storage_key, table: backend_table(backend)} + end + + defp periodic_sync(pid) do + send(pid, {:durable, :sync}) + :sys.get_state(pid) + end + defp assert_call_sync_failure(pid, request, reason) do monitor_ref = Process.monitor(pid) expected_exit = fatal_sync_exit(reason) @@ -824,6 +859,50 @@ defmodule DurableServerTest do end end + defmodule DirtyTrackingServer do + use DurableServer, vsn: 1 + + def dump_state(state) do + if probe_pid = state[:probe_pid] do + send(probe_pid, {:dirty_tracking_dump, self(), state.count}) + end + + Map.take(state, [:count]) + end + + def load_state(_old_vsn, state), do: state + + def init(state, info) do + {:ok, Map.merge(state, %{probe_pid: info.probe_pid, transient: 0, after_sync: nil}), + auto_sync: info.auto_sync} + end + + def handle_call(:get_state, _from, state), do: {:reply, state, state} + def handle_call(:sync_same, _from, state), do: {:reply, :ok, state, :sync} + + def handle_call({:set, key, value}, _from, state) do + {:reply, :ok, Map.put(state, key, value)} + end + + def handle_call({:continue_count, count}, _from, state) do + {:reply, :ok, state, {:continue, {:set_count, count}}} + end + + def handle_cast({:set_count, count}, state), do: {:noreply, %{state | count: count}} + def handle_info({:set_count, count}, state), do: {:noreply, %{state | count: count}} + def handle_continue({:set_count, count}, state), do: {:noreply, %{state | count: count}} + + def handle_sync(_info, _old_state, %{after_sync: :durable} = state) do + {:ok, %{state | count: state.count + 1, after_sync: nil}} + end + + def handle_sync(_info, _old_state, %{after_sync: :transient} = state) do + {:ok, %{state | transient: state.transient + 1, after_sync: nil}} + end + + def handle_sync(_info, _old_state, state), do: {:ok, state} + end + defmodule PeriodicSyncServer do use DurableServer, vsn: 1 @@ -2463,6 +2542,209 @@ defmodule DurableServerTest do end end + describe "dirty tracking" do + @describetag :dirty_tracking + + test "idle ticks and unchanged callbacks skip dumping and storage writes" do + %{pid: pid, storage_key: storage_key} = start_dirty_tracking_server() + + for _ <- 1..3 do + assert %{count: 0} = GenServer.call(pid, :get_state) + assert :ok = GenServer.call(pid, {:set, :count, 0}) + GenServer.cast(pid, {:set_count, 0}) + send(pid, {:set_count, 0}) + assert :ok = GenServer.call(pid, {:continue_count, 0}) + periodic_sync(pid) + end + + refute_received {:dirty_tracking_dump, ^pid, _count} + refute_received {:dirty_tracking_put, ^storage_key, _result} + end + + test "unchanged callbacks preserve pending changes from every callback type" do + %{pid: pid, backend: backend, storage_key: storage_key} = start_dirty_tracking_server() + + updates = [ + fn -> GenServer.call(pid, {:set, :count, 1}) end, + fn -> GenServer.cast(pid, {:set_count, 2}) end, + fn -> send(pid, {:set_count, 3}) end, + fn -> GenServer.call(pid, {:continue_count, 4}) end + ] + + for {update, count} <- Enum.with_index(updates, 1) do + update.() + assert %{count: ^count} = GenServer.call(pid, :get_state) + periodic_sync(pid) + + assert_received {:dirty_tracking_dump, ^pid, ^count} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert {:ok, %{body: %StoredState{state: %{count: ^count}}}} = + StorageBackend.get_object(backend, storage_key) + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + refute_received {:dirty_tracking_put, ^storage_key, _result} + end + end + + test "transient changes and reverted durable changes need only one check" do + %{pid: pid, storage_key: storage_key} = start_dirty_tracking_server() + + assert :ok = GenServer.call(pid, {:set, :transient, 1}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + + assert :ok = GenServer.call(pid, {:set, :count, 1}) + assert :ok = GenServer.call(pid, {:set, :count, 0}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + end + + test "state comparison distinguishes integers from floats" do + %{pid: pid, backend: backend, storage_key: storage_key} = start_dirty_tracking_server() + float_count = 0.0 + + assert :ok = GenServer.call(pid, {:set, :count, float_count}) + periodic_sync(pid) + + assert_received {:dirty_tracking_dump, ^pid, ^float_count} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert {:ok, %{body: %StoredState{state: %{count: ^float_count}}}} = + StorageBackend.get_object(backend, storage_key) + end + + test "automatic sync skips unchanged callbacks after checking changes" do + %{pid: pid, storage_key: storage_key} = start_dirty_tracking_server(true) + + assert %{count: 0} = GenServer.call(pid, :get_state) + refute_received {:dirty_tracking_dump, ^pid, _count} + + assert :ok = GenServer.call(pid, {:set, :transient, 1}) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + assert :ok = GenServer.call(pid, {:set, :count, 1}) + assert_received {:dirty_tracking_dump, ^pid, 1} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert %{count: 1} = GenServer.call(pid, :get_state) + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + refute_received {:dirty_tracking_put, ^storage_key, _result} + end + + test "failed writes remain pending across unchanged callbacks" do + %{pid: pid, backend: backend, storage_key: storage_key, table: table} = + start_dirty_tracking_server() + + put_backend_write_override(table, storage_key, {:error, :unavailable}) + assert :ok = GenServer.call(pid, {:set, :count, 1}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 1} + assert_received {:dirty_tracking_put, ^storage_key, {:error, :unavailable}} + + assert {:ok, %{body: %StoredState{state: %{count: 0}}}} = + StorageBackend.get_object(backend, storage_key) + + assert %{count: 1} = GenServer.call(pid, :get_state) + clear_backend_write_override(table, storage_key) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 1} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert {:ok, %{body: %StoredState{state: %{count: 1}}}} = + StorageBackend.get_object(backend, storage_key) + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + end + + test "durable changes returned by handle_sync remain pending for another write" do + %{pid: pid, backend: backend, storage_key: storage_key} = start_dirty_tracking_server() + + assert :ok = GenServer.call(pid, {:set, :after_sync, :durable}) + assert :ok = GenServer.call(pid, {:set, :count, 1}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 1} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + assert %{count: 2} = GenServer.call(pid, :get_state) + + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 2} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert {:ok, %{body: %StoredState{state: %{count: 2}}}} = + StorageBackend.get_object(backend, storage_key) + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + end + + test "transient changes returned by handle_sync clear after one noop check" do + %{pid: pid, storage_key: storage_key} = start_dirty_tracking_server() + + assert :ok = GenServer.call(pid, {:set, :after_sync, :transient}) + assert :ok = GenServer.call(pid, {:set, :count, 1}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 1} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + assert %{count: 1, transient: 1} = GenServer.call(pid, :get_state) + + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 1} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + end + + test "explicit sync and final status writes still evaluate clean state" do + %{pid: pid, backend: backend, storage_key: storage_key} = start_dirty_tracking_server() + + assert :ok = GenServer.call(pid, :sync_same) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + assert :ok = GenServer.stop(pid, :shutdown) + assert_received {:dirty_tracking_dump, ^pid, 0} + assert_received {:dirty_tracking_put, ^storage_key, {:ok, _object}} + + assert {:ok, %{body: %StoredState{meta: %Meta{status: :stopped_graceful}}}} = + StorageBackend.get_object(backend, storage_key) + end + + test "code upgrades invalidate the check even when user state is unchanged" do + %{pid: pid, storage_key: storage_key} = start_dirty_tracking_server() + + :ok = :sys.suspend(pid) + :ok = :sys.change_code(pid, DurableServer, 1, :noop) + :ok = :sys.resume(pid) + assert %{count: 0} = GenServer.call(pid, :get_state) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + # Exercise lazy version migration on a tick without a user callback. + :sys.replace_state(pid, &%{&1 | vsn: 0}) + periodic_sync(pid) + assert_received {:dirty_tracking_dump, ^pid, 0} + refute_received {:dirty_tracking_put, ^storage_key, _result} + + periodic_sync(pid) + refute_received {:dirty_tracking_dump, ^pid, _count} + end + end + describe "periodic sync functionality" do @tag :slow test "periodically syncs state based on sync_every_ms", %{ From 67bdc9db382f4c98e854e84fab47d7046d4cc3bd Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Wed, 7 Oct 2026 08:26:17 -0400 Subject: [PATCH 2/3] Balance new child placement across local and remote nodes --- CHANGELOG.md | 1 + README.md | 2 +- lib/durable_server.ex | 25 ++-- lib/durable_server/lifecycle_manager.ex | 21 ++- lib/durable_server/supervisor.ex | 131 +++++++++++++----- .../balanced_placement_test.exs | 129 +++++++++++++++++ test/durable_server/remote_placement_test.exs | 118 +++++++++++++++- 7 files changed, 369 insertions(+), 58 deletions(-) create mode 100644 test/durable_server/balanced_placement_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 800b01d..e6a92a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased +- Balance new child starts using the existing eligible-node ranking, considering live local capacity alongside remote heartbeat capacity instead of filling local first. Explicit local starts, restart/rehome, and sticky-placement behavior are unchanged. - Skip dumping, serialization, and hashing during periodic and automatic sync when callback state is exactly unchanged and no earlier change is pending. Runtime state changes still use `dump_state/1` to determine whether a storage write is needed; explicit sync and lifecycle writes continue to evaluate the dump. - Resolve duplicate DurableServer ownership by a persisted, monotonically increasing lock epoch. Group now keeps the latest successful object-store claimant and retires only stale owners instead of killing every claimant and causing repeated anti-entropy restart loops. - Enforce cumulative sticky-placement gates during request-driven remote placement so an existing server cannot move to a fallback node before that level unlocks. diff --git a/README.md b/README.md index 7d4cb6f..63d205c 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ It implements fault-tolerant, stateful processes that can survive node failures, - **Durable state**: Automatically persists state to storage with configurable sync intervals - **Cluster coordination**: Uses distributed registry for process discovery and health monitoring -- **Capacity-aware placement**: Monitors CPU, memory, and disk usage to route new processes to nodes with available capacity +- **Capacity-aware placement**: Ranks local and remote nodes together by utilization, spreading new processes before local fills while respecting child-count and resource limits - **Sticky placement**: Environment variable-based placement preferences (e.g., same machine, same region, etc.) with time-gated fallback to ensure servers restart on preferred nodes when possible - **Automatic recovery**: Failed processes are detected and restarted across the cluster - **Graceful shutdown**: Ensures state is synchronized before termination diff --git a/lib/durable_server.ex b/lib/durable_server.ex index 59d221b..8d62431 100644 --- a/lib/durable_server.ex +++ b/lib/durable_server.ex @@ -274,20 +274,15 @@ defmodule DurableServer do ## Capacity-Aware Placement - DurableServers support automatic capacity-aware placement with remote fallback. + New children use the existing least-busy eligible-node ranking, with local considered + alongside remote nodes instead of filling local capacity first. Ranking uses the + highest utilization across configured child-count, CPU, and memory limits. Full, + draining, or resource-limited nodes are excluded; connected nodes are preferred. - ### Local Placement (Default) - - When starting a child, the local node is tried first. If capacity limits are exceeded, - remote placement is attempted automatically. - - ### Remote Placement - - If local capacity is exhausted, DurableServer automatically tries remote nodes: - - 1. **Same-region nodes first** - Prioritizes nodes in the same region for lower latency - 2. **Least busy nodes** - Selects nodes with the lowest utilization across all limits - 3. **Configurable retries** - Default 3 remote nodes tried, configurable via `max_placement_retries` + Local counts include in-flight start reservations. Remote load remains heartbeat-based, + so placement is approximate rather than strictly even during bursts. Admission checks + on the selected node still enforce its limits. Restart, rehome, and sticky-placement + behavior is unchanged. ### Capacity Limits @@ -311,14 +306,14 @@ defmodule DurableServer do Control remote placement behavior per start_child call: - # Default: Try local, then up to 3 remote nodes + # Default: Rank local alongside up to 3 remote nodes DurableServer.Supervisor.start_child(sup, {MyServer, key: "user_1", initial_state: %{}}) # Local only, no remote fallback DurableServer.Supervisor.start_child(sup, {MyServer, key: "user_1", initial_state: %{}}, max_placement_retries: 0) - # Try local, then up to 5 remote nodes + # Rank local alongside up to 5 remote nodes DurableServer.Supervisor.start_child(sup, {MyServer, key: "user_1", initial_state: %{}}, max_placement_retries: 5) diff --git a/lib/durable_server/lifecycle_manager.ex b/lib/durable_server/lifecycle_manager.ex index 3bb1893..1023fd8 100644 --- a/lib/durable_server/lifecycle_manager.ex +++ b/lib/durable_server/lifecycle_manager.ex @@ -3329,12 +3329,13 @@ defmodule DurableServer.LifecycleManager do (global capacity, module capacity, CPU, memory). Nodes with lower utilization are prioritized for better load distribution. - Always excludes the local node since we only lookup eligible remote nodes - after local placement fails. + Excludes the local node unless `:include_local` is set. When included, local + capacity and resources are read directly instead of using its last heartbeat. ## Options - `:limit` - Maximum number of nodes to return (default: 3) + - `:include_local` - Include the local node using live capacity (default: false) - `:key` - The server key, used to load augmented sticky placement preferences and timing metadata ## Examples @@ -3357,6 +3358,7 @@ defmodule DurableServer.LifecycleManager do limit = Keyword.get(opts, :limit, 3) key = Keyword.get(opts, :key) my_node = Node.self() + include_local = Keyword.get(opts, :include_local, false) # Get both the effective placement and its persisted metadata. Candidate # selection needs the metadata heartbeat to enforce cumulative time gates. @@ -3374,6 +3376,19 @@ defmodule DurableServer.LifecycleManager do # Liveness is ALWAYS computed from timestamp, never from mere Group presence. merged_nodes = merge_heartbeat_sources(supervisor_name, heartbeat_table, now) + merged_nodes = + if include_local do + local_entry = + {to_string(my_node), DurableServer.Supervisor.node_ref(supervisor_name), now, + DurableServer.Supervisor.current_capacity(supervisor_name), + calculate_resource_map(supervisor_name), get_my_env_vars(supervisor_name), + %{"draining" => check_shutting_down(supervisor_name) != :ok}} + + [local_entry | Enum.reject(merged_nodes, &(elem(&1, 0) == to_string(my_node)))] + else + merged_nodes + end + merged_nodes |> Enum.map(fn {node_str, node_ref, timestamp, capacity, resources, env_vars, heartbeat_meta} -> @@ -3409,7 +3424,7 @@ defmodule DurableServer.LifecycleManager do end) |> Enum.reject(&is_nil/1) |> Enum.filter(fn {node, health, matching_level, _timestamp} -> - node != my_node and + (include_local or node != my_node) and can_node_accept_module?(health, module, matching_level: matching_level) and sticky_candidate_gate_open?( sticky_placement, diff --git a/lib/durable_server/supervisor.ex b/lib/durable_server/supervisor.ex index a8c8646..105a4ef 100644 --- a/lib/durable_server/supervisor.ex +++ b/lib/durable_server/supervisor.ex @@ -760,6 +760,10 @@ defmodule DurableServer.Supervisor do @doc """ Starts a DurableServer child process under this supervisor. + New children are placed using the existing least-busy eligible-node ranking, + with the local node considered alongside remote nodes. Explicit local starts, + automatic restarts, and sticky placement retain their existing behavior. + The child spec is `{Module, key: key, initial_state: initial_state}`. `:initial_state` is required and must be a map. Before the first `init/1` or `init/2` call, DurableServer passes it through the module's `dump_state/1`, @@ -772,9 +776,10 @@ defmodule DurableServer.Supervisor do - `:local_only` - When `true`, the child will only be started on the local node. If the local node is at capacity, returns `{:error, {:capacity_limit, reason}}` instead of attempting remote placement. Default: `false`. - - `:max_placement_retries` - Maximum number of remote nodes to try when local - placement fails due to capacity limits. Default: `3`. Ignored when `local_only: true`. - - `:placement_timeout` - Maximum time in milliseconds to keep retrying remote placement. + - `:max_placement_retries` - Maximum number of remote nodes to try per placement + round, in addition to local. Set to `0` for local-only placement. Default: `3`. + Ignored when `local_only: true`. + - `:placement_timeout` - Maximum time in milliseconds to keep retrying placement. If all placement attempts fail, the caller retries with fresh eligible nodes every #{@placement_retry_interval}ms until the deadline. Useful during rolling deploys when nodes are temporarily unavailable. Set to `nil` to disable. Default: `#{@default_placement_timeout}`ms. @@ -934,28 +939,57 @@ defmodule DurableServer.Supervisor do check_existing(supervisor, init_arg, boot_info, Keyword.get(opts, :existing, false)) do child_spec = {module, init_arg, boot_info} - case do_start_child(supervisor, child_spec, 0, caller_deadline_ms, reply_to) do - {:ok, result} -> - {:ok, result} + # New starts rank local alongside the other nodes. Keep targeted/preloaded + # starts and already-running children on their existing paths. + ranked_start? = + max_placement_retries > 0 and is_nil(boot_info) and + is_nil(lookup(supervisor, Keyword.fetch!(init_arg, :key))) + + if ranked_start? do + # A zero placement timeout means one attempt round, not a zero-length + # bootstrap deadline. The caller timeout still bounds that round. + placement_deadline_ms = + if placement_timeout == 0, + do: caller_deadline_ms, + else: + earlier_deadline( + caller_deadline_ms, + deadline_after_optional_timeout(placement_timeout) + ) - {:error, {:capacity_limit, reason}} when max_placement_retries > 0 -> - Logger.info(""" - DurableServer local capacity exceeded for #{inspect(module)} on #{Node.self()} - Reason: #{inspect(reason)} - Attempting remote placement (max retries: #{max_placement_retries}) - """) + try_placement_with_retry( + supervisor, + child_spec, + max_placement_retries, + placement_deadline_ms, + include_local: true, + reply_to: reply_to, + retry: placement_timeout != 0 + ) + else + case do_start_child(supervisor, child_spec, 0, caller_deadline_ms, reply_to) do + {:ok, result} -> + {:ok, result} - placement_deadline_ms = deadline_after_optional_timeout(placement_timeout) + {:error, {:capacity_limit, reason}} when max_placement_retries > 0 -> + Logger.info(""" + DurableServer local capacity exceeded for #{inspect(module)} on #{Node.self()} + Reason: #{inspect(reason)} + Attempting remote placement (max retries: #{max_placement_retries}) + """) - try_remote_placement_with_retry( - supervisor, - child_spec, - max_placement_retries, - earlier_deadline(caller_deadline_ms, placement_deadline_ms) - ) + placement_deadline_ms = deadline_after_optional_timeout(placement_timeout) - error -> - error + try_placement_with_retry( + supervisor, + child_spec, + max_placement_retries, + earlier_deadline(caller_deadline_ms, placement_deadline_ms) + ) + + error -> + error + end end end after @@ -1630,11 +1664,12 @@ defmodule DurableServer.Supervisor do defp restart_claim_race_final_retry?(_retries, _remaining_ms), do: false - defp try_remote_placement( + defp try_placement( supervisor, {module, init_arg, boot_info} = child_spec, max_retries, - deadline + deadline, + opts \\ [] ) do key = Keyword.fetch!(init_arg, :key) @@ -1655,6 +1690,7 @@ defmodule DurableServer.Supervisor do eligible_nodes = LifecycleManager.find_eligible_nodes(supervisor, module, + include_local: Keyword.get(opts, :include_local, false), limit: candidate_limit, key: key, sticky_placement: sticky_placement, @@ -1673,6 +1709,8 @@ defmodule DurableServer.Supervisor do nodes -> try_nodes(supervisor, child_spec, nodes, + include_local: Keyword.get(opts, :include_local, false), + reply_to: Keyword.get(opts, :reply_to), key: key, sticky_placement: sticky_placement, sticky_meta: sticky_meta, @@ -1681,13 +1719,13 @@ defmodule DurableServer.Supervisor do end end - # Prioritizes remote placement targets to avoid timeout storms: + # Prioritizes placement targets to avoid timeout storms: # 1) Prefer connected nodes first # 2) Skip nodes currently in timeout cooldown # 3) If no connected targets remain, allow disconnected nodes as fallback defp prioritize_placement_nodes(nodes, supervisor, max_retries) when is_list(nodes) and is_atom(supervisor) and is_integer(max_retries) do - connected_set = Node.list() |> MapSet.new() + connected_set = [node() | Node.list()] |> MapSet.new() {connected_nodes, disconnected_nodes} = Enum.split_with(nodes, fn node -> MapSet.member?(connected_set, node) end) @@ -1711,7 +1749,9 @@ defmodule DurableServer.Supervisor do [] -> disconnected_candidates end - Enum.take(candidates, max_retries) + # Local is an extra candidate, not part of the remote attempt budget. + remote_candidates = candidates |> Enum.reject(&(&1 == node())) |> Enum.take(max_retries) + Enum.filter(candidates, &(&1 == node() or &1 in remote_candidates)) end defp placement_node_in_timeout_cooldown?(supervisor, node) @@ -1756,17 +1796,17 @@ defmodule DurableServer.Supervisor do _ -> :ok end - defp try_remote_placement_with_retry(supervisor, child_spec, max_retries, deadline) do - case try_remote_placement(supervisor, child_spec, max_retries, deadline) do + defp try_placement_with_retry(supervisor, child_spec, max_retries, deadline, opts \\ []) do + case try_placement(supervisor, child_spec, max_retries, deadline, opts) do {:ok, result} -> {:ok, result} {:error, {:capacity_limit, reason}} = error when reason in [:no_available_nodes, :all_placement_attempts_failed] -> - if deadline != nil and + if Keyword.get(opts, :retry, true) and deadline != nil and System.monotonic_time(:millisecond) + @placement_retry_interval < deadline do Process.sleep(@placement_retry_interval) - try_remote_placement_with_retry(supervisor, child_spec, max_retries, deadline) + try_placement_with_retry(supervisor, child_spec, max_retries, deadline, opts) else error end @@ -1860,6 +1900,23 @@ defmodule DurableServer.Supervisor do {:error, {:capacity_limit, :all_placement_attempts_failed}} end + defp try_nodes(supervisor, child_spec, [target | rest], placement_opts) + when target == node() do + case do_start_child( + supervisor, + child_spec, + 0, + Keyword.get(placement_opts, :deadline), + Keyword.fetch!(placement_opts, :reply_to) + ) do + {:error, {:capacity_limit, _reason}} -> + try_nodes(supervisor, child_spec, rest, placement_opts) + + result -> + result + end + end + defp try_nodes( supervisor, {module, _init_arg, _boot_info} = child_spec, @@ -1949,6 +2006,7 @@ defmodule DurableServer.Supervisor do fresh_nodes = LifecycleManager.find_eligible_nodes(supervisor, module, limit: 3, + include_local: Keyword.get(placement_opts, :include_local, false), key: Keyword.get(placement_opts, :key), sticky_placement: Keyword.get(placement_opts, :sticky_placement), sticky_meta: Keyword.get(placement_opts, :sticky_meta) @@ -2061,9 +2119,10 @@ defmodule DurableServer.Supervisor do Skips sticky placement preferences and never attempts remote placement. If the local node is at capacity, returns `{:error, {:capacity_limit, reason}}`. Default: `false`. - - `:max_placement_retries` - Maximum number of remote nodes to try when local - placement fails due to capacity limits. Default: `3`. Ignored when `local_only: true`. - - `:placement_timeout` - Maximum time in milliseconds to keep retrying remote placement. + - `:max_placement_retries` - Maximum number of remote nodes to try per placement + round, in addition to local. Set to `0` for local-only placement. Default: `3`. + Ignored when `local_only: true`. + - `:placement_timeout` - Maximum time in milliseconds to keep retrying placement. When set, if all placement attempts fail, retries with fresh eligible nodes every #{@placement_retry_interval}ms until the deadline. Default: `nil` (no retry). - `:timeout` - Maximum total time in milliseconds to wait for the process to be @@ -2245,7 +2304,7 @@ defmodule DurableServer.Supervisor do else case sticky_placement do nil -> - # No sticky placement, proceed with normal local-first logic + # No sticky placement, proceed with normal placement. {_should_skip_local = false, _is_sticky_local = false, _matching_level = nil} [%{env_var: :any, value: :any} | _] -> @@ -2321,7 +2380,7 @@ defmodule DurableServer.Supervisor do ) true -> - # Normal flow: try local first, then remote if capacity exceeded + # Rank new children; persisted children retain their local/sticky path. start_opts = opts |> Keyword.delete(:existing) @@ -2636,7 +2695,7 @@ defmodule DurableServer.Supervisor do caller_deadline_ms ) do # First attempt remote placement (single round, no retry loop — we handle retry here) - case try_remote_placement(supervisor, child_spec, 3, placement_deadline_ms) do + case try_placement(supervisor, child_spec, 3, placement_deadline_ms) do {:ok, result} -> {:ok, result} diff --git a/test/durable_server/balanced_placement_test.exs b/test/durable_server/balanced_placement_test.exs new file mode 100644 index 0000000..e33d62f --- /dev/null +++ b/test/durable_server/balanced_placement_test.exs @@ -0,0 +1,129 @@ +defmodule DurableServer.BalancedPlacementTest do + use ExUnit.Case, async: false + import DurableServer.TestHelper + alias DurableServer.{Supervisor, TestCounterServer} + + @moduletag :capture_log + + setup_all do + unless Node.alive?() do + {:ok, _} = Node.start(:"placement_test_#{System.unique_integer([:positive])}", :shortnames) + end + + {:ok, peer, remote} = + :peer.start(%{name: :"placement_peer_#{System.unique_integer([:positive])}"}) + + on_exit(fn -> + if Process.alive?(peer), do: :peer.stop(peer) + end) + + :ok = :erpc.call(remote, :code, :add_paths, [:code.get_path()]) + {:ok, _} = :erpc.call(remote, Application, :ensure_all_started, [:durable_server]) + {:ok, remote: remote} + end + + test "new starts balance before local fills and retain local fallback", %{remote: remote} do + name = :"balanced_supervisor_#{System.unique_integer([:positive])}" + + opts = [ + name: name, + prefix: "balanced_placement_#{System.unique_integer([:positive])}/", + object_store: test_object_store_opts(), + max_children: %{TestCounterServer => 100} + ] + + start_supervised!({Supervisor, opts}) + + {:ok, _} = + :erpc.call(remote, Elixir.Supervisor, :start_child, [ + DurableServer.AppSupervisor, + {Supervisor, opts} + ]) + + on_exit(fn -> + :erpc.call(remote, Elixir.Supervisor, :terminate_child, [DurableServer.AppSupervisor, name]) + :erpc.call(remote, Elixir.Supervisor, :delete_child, [DurableServer.AppSupervisor, name]) + end) + + placements = + for i <- 1..10 do + # Deterministic heartbeat snapshots isolate the placement decision from + # gossip timing. Production remote counts remain heartbeat-based. + refresh_remote_capacity(name, remote) + + assert {:ok, {pid, _}} = + Supervisor.start_child( + name, + {TestCounterServer, key: "balanced-#{i}", initial_state: %{count: 0}}, + placement_timeout: 0 + ) + + node(pid) + end + + assert Enum.frequencies(placements) == %{node() => 5, remote => 5} + + # Existing children are returned without routing them to a less busy node. + assert {:ok, {existing, _}} = + Supervisor.ensure_started_child(name, { + TestCounterServer, + key: "balanced-1", initial_state: %{count: 0} + }) + + assert {:error, {:already_started, {^existing, _}}} = + Supervisor.start_child(name, { + TestCounterServer, + key: "balanced-1", initial_state: %{count: 0} + }) + + for {key, start_opts} <- [ + {"forced-local", [local_only: true]}, + {"targeted-local", [max_placement_retries: 0]} + ] do + assert {:ok, {pid, _}} = + Supervisor.start_child( + name, + {TestCounterServer, key: key, initial_state: %{count: 0}}, + start_opts + ) + + assert node(pid) == node() + end + + refresh_remote_capacity(name, remote) + + assert {:ok, {pid, _}} = + Supervisor.ensure_started_child(name, { + TestCounterServer, + key: "ensure-new", initial_state: %{count: 0} + }) + + assert node(pid) == remote + + # The remote snapshot says it can accept a child, but admission now rejects + # it. Local must still be tried, even with a one-remote-node retry budget. + refresh_remote_capacity(name, remote) + %{ets_table: remote_table} = :erpc.call(remote, Supervisor, :__get_config__, [name]) + :erpc.call(remote, :ets, :insert, [remote_table, {:shutting_down, true}]) + + assert {:ok, {fallback, _}} = + Supervisor.start_child( + name, + {TestCounterServer, key: "local-fallback", initial_state: %{count: 0}}, + max_placement_retries: 1, + placement_timeout: 0 + ) + + assert node(fallback) == node() + end + + defp refresh_remote_capacity(supervisor, remote) do + capacity = :erpc.call(remote, Supervisor, :current_capacity, [supervisor]) + node_ref = :erpc.call(remote, Supervisor, :node_ref, [supervisor]) + + :ets.insert( + :"durable_server_heartbeats_#{supervisor}", + {to_string(remote), node_ref, System.system_time(:millisecond), capacity, nil, %{}, %{}} + ) + end +end diff --git a/test/durable_server/remote_placement_test.exs b/test/durable_server/remote_placement_test.exs index 76e7c5a..2e483a7 100644 --- a/test/durable_server/remote_placement_test.exs +++ b/test/durable_server/remote_placement_test.exs @@ -84,7 +84,7 @@ defmodule DurableServer.RemotePlacementTest do ) end - test "always excludes local node", %{ + test "excludes local node by default", %{ supervisor_name: supervisor_name, prefix: prefix } do @@ -97,7 +97,7 @@ defmodule DurableServer.RemotePlacementTest do max_children: %{:total => 2}} ) - # Local node should never be included (function is only called after local placement fails) + # Remote-only callers (restart/rehome) retain their existing candidate set. nodes = DurableServer.LifecycleManager.find_eligible_nodes( supervisor_name, @@ -107,6 +107,117 @@ defmodule DurableServer.RemotePlacementTest do refute Node.self() in nodes end + test "ranks live local reservations alongside remote capacity", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, + prefix: prefix, + object_store: test_object_store_opts(), + max_children: %{RemotePlacementTestServer => 3}} + ) + + local = node() + remote = :ranked_remote@test + heartbeat_table = :"durable_server_heartbeats_#{supervisor_name}" + now = System.system_time(:millisecond) + + # A stale local count must not hide either free slots or pending starts. + :ets.insert(heartbeat_table, [ + {to_string(local), 1, now, %{RemotePlacementTestServer => %{current: 3, limit: 3}}, nil, + %{}, %{}}, + {to_string(remote), 2, now, %{RemotePlacementTestServer => %{current: 1, limit: 3}}, nil, + %{}, %{}} + ]) + + candidates = fn -> + LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + include_local: true + ) + end + + assert candidates.() == [local, remote] + + {:ok, first} = LifecycleManager.reserve_capacity(supervisor_name, RemotePlacementTestServer) + + {:ok, second} = + LifecycleManager.reserve_capacity(supervisor_name, RemotePlacementTestServer) + + assert candidates.() == [remote, local] + + {:ok, third} = LifecycleManager.reserve_capacity(supervisor_name, RemotePlacementTestServer) + assert candidates.() == [remote] + + Enum.each([first, second, third], &LifecycleManager.cancel_capacity_reservation/1) + end + + test "local inclusion still respects zero module capacity", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, + prefix: prefix, + object_store: test_object_store_opts(), + max_children: %{RemotePlacementTestServer => 0}} + ) + + assert [] == + LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + include_local: true + ) + end + + test "local inclusion respects live resource and draining limits", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, + prefix: prefix, + object_store: test_object_store_opts(), + max_memory: 80} + ) + + %{ets_table: table} = DurableServer.Supervisor.__get_config__(supervisor_name) + + candidates = fn -> + LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + include_local: true + ) + end + + :ets.insert(table, {:resource_metrics, {nil, 90, nil, System.system_time(:millisecond)}}) + assert candidates.() == [] + + :ets.insert(table, {:resource_metrics, {nil, 20, nil, System.system_time(:millisecond)}}) + assert candidates.() == [node()] + + :ets.insert(table, {:shutting_down, true}) + assert candidates.() == [] + end + + test "local inclusion retains sticky preference gates", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, prefix: prefix, object_store: test_object_store_opts()} + ) + + assert [] == + LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + include_local: true, + sticky_placement: [%{env_var: "OTHER_MACHINE", value: "remote"}], + sticky_meta: nil + ) + end + test "respects limit option", %{supervisor_name: supervisor_name, prefix: prefix} do # Start supervisor start_supervised!( @@ -270,7 +381,8 @@ defmodule DurableServer.RemotePlacementTest do DurableServer.Supervisor.start_child( supervisor_name, {RemotePlacementTestServer, key: "key1", initial_state: %{}}, - max_placement_retries: 3 + max_placement_retries: 3, + placement_timeout: 0 ) assert is_pid(pid) From 948fbd02bbcff365437edcfef6af9834cc481e3e Mon Sep 17 00:00:00 2001 From: Chris McCord Date: Wed, 7 Oct 2026 08:45:46 -0400 Subject: [PATCH 3/3] Preserve eligible local placement beyond the candidate pool limit --- CHANGELOG.md | 2 +- lib/durable_server/lifecycle_manager.ex | 12 ++- test/durable_server/remote_placement_test.exs | 97 +++++++++++++++++++ 3 files changed, 108 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6a92a5..ef55ccd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Unreleased -- Balance new child starts using the existing eligible-node ranking, considering live local capacity alongside remote heartbeat capacity instead of filling local first. Explicit local starts, restart/rehome, and sticky-placement behavior are unchanged. +- Balance new child starts using the existing eligible-node ranking, considering live local capacity alongside remote heartbeat capacity instead of filling local first. Eligible local remains a candidate even when it ranks beyond the remote candidate-pool limit. Explicit local starts, restart/rehome, and sticky-placement behavior are unchanged. - Skip dumping, serialization, and hashing during periodic and automatic sync when callback state is exactly unchanged and no earlier change is pending. Runtime state changes still use `dump_state/1` to determine whether a storage write is needed; explicit sync and lifecycle writes continue to evaluate the dump. - Resolve duplicate DurableServer ownership by a persisted, monotonically increasing lock epoch. Group now keeps the latest successful object-store claimant and retires only stale owners instead of killing every claimant and causing repeated anti-entropy restart loops. - Enforce cumulative sticky-placement gates during request-driven remote placement so an existing server cannot move to a fallback node before that level unlocks. diff --git a/lib/durable_server/lifecycle_manager.ex b/lib/durable_server/lifecycle_manager.ex index 1023fd8..5b96b53 100644 --- a/lib/durable_server/lifecycle_manager.ex +++ b/lib/durable_server/lifecycle_manager.ex @@ -3334,7 +3334,8 @@ defmodule DurableServer.LifecycleManager do ## Options - - `:limit` - Maximum number of nodes to return (default: 3) + - `:limit` - Maximum number of ranked candidates to return (default: 3). + With `:include_local`, an eligible local node is also retained if it falls beyond this limit. - `:include_local` - Include the local node using live capacity (default: false) - `:key` - The server key, used to load augmented sticky placement preferences and timing metadata @@ -3439,8 +3440,15 @@ defmodule DurableServer.LifecycleManager do staleness = -timestamp {level_priority, busyness, staleness} end) - |> Enum.take(limit) |> Enum.map(fn {node, _health, _matching_level, _timestamp} -> node end) + |> Enum.split(limit) + |> then(fn {candidates, rest} -> + # Keep local available as a fallback even when busier than the whole + # remote pool. It has already passed the same eligibility checks. + if include_local, + do: candidates ++ Enum.filter(rest, &(&1 == my_node)), + else: candidates + end) end end diff --git a/test/durable_server/remote_placement_test.exs b/test/durable_server/remote_placement_test.exs index 2e483a7..bbf396f 100644 --- a/test/durable_server/remote_placement_test.exs +++ b/test/durable_server/remote_placement_test.exs @@ -171,6 +171,49 @@ defmodule DurableServer.RemotePlacementTest do ) end + test "candidate limit preserves eligible local without changing rank or duplicating it", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, + prefix: prefix, + object_store: test_object_store_opts(), + max_children: %{total: 2}} + ) + + remotes = add_ranked_remote_heartbeats(supervisor_name, 13) + + candidates = fn -> + LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + include_local: true, + limit: 12 + ) + end + + # Local stays first when least busy and is not added a second time. + assert candidates.() == [node() | Enum.take(remotes, 11)] + + {:ok, first} = LifecycleManager.reserve_capacity(supervisor_name, RemotePlacementTestServer) + + # All 13 remotes now rank ahead of local. Retain local after the capped + # remote pool without promoting it ahead of less busy candidates. + assert candidates.() == Enum.take(remotes, 12) ++ [node()] + + assert LifecycleManager.find_eligible_nodes(supervisor_name, RemotePlacementTestServer, + limit: 12 + ) == Enum.take(remotes, 12) + + {:ok, second} = + LifecycleManager.reserve_capacity(supervisor_name, RemotePlacementTestServer) + + # Retention must not resurrect an ineligible (full) local node. + assert candidates.() == Enum.take(remotes, 12) + + Enum.each([first, second], &LifecycleManager.cancel_capacity_reservation/1) + end + test "local inclusion respects live resource and draining limits", %{ supervisor_name: supervisor_name, prefix: prefix @@ -364,6 +407,46 @@ defmodule DurableServer.RemotePlacementTest do end describe "start_child with max_placement_retries" do + test "retains local when a full candidate pool of less busy remotes is in cooldown", %{ + supervisor_name: supervisor_name, + prefix: prefix + } do + start_supervised!( + {DurableServer.Supervisor, + name: supervisor_name, + prefix: prefix, + object_store: test_object_store_opts(), + max_children: %{total: 2}} + ) + + assert {:ok, _} = + DurableServer.Supervisor.start_child( + supervisor_name, + {RemotePlacementTestServer, key: "local-seed", initial_state: %{}}, + local_only: true + ) + + %{circuit_breaker: breaker} = DurableServer.Supervisor.__get_config__(supervisor_name) + + for remote <- add_ranked_remote_heartbeats(supervisor_name, 13) do + :ok = + DurableServer.CircuitBreaker.trip_placement_node_timeout_circuit_breaker( + breaker, + to_string(remote), + 60_000 + ) + end + + assert {:ok, {pid, _}} = + DurableServer.Supervisor.start_child( + supervisor_name, + {RemotePlacementTestServer, key: "local-fallback", initial_state: %{}}, + placement_timeout: 0 + ) + + assert node(pid) == node() + end + test "succeeds locally when capacity available", %{ supervisor_name: supervisor_name, prefix: prefix @@ -938,4 +1021,18 @@ defmodule DurableServer.RemotePlacementTest do ) end end + + defp add_ranked_remote_heartbeats(supervisor_name, count) do + remotes = for i <- 1..count, do: :"placement_pool_remote_#{i}@test" + now = System.system_time(:millisecond) + + entries = + for {remote, current} <- Enum.with_index(remotes, 1) do + {to_string(remote), current, now, %{total: %{current: current, limit: 100}}, nil, %{}, + %{}} + end + + :ets.insert(:"durable_server_heartbeats_#{supervisor_name}", entries) + remotes + end end