Skip to content

Commit b8be884

Browse files
authored
fix(sync-service): reduce live request wait memory (#4680)
## Motivation Live shape requests spend most of their long-poll lifetime parked in a receive while waiting for either a shape change or timeout. Under high concurrency, parking inside the larger request-handling function kept more heap live than the small wait state actually needed, which made the per-request memory floor significant. ## Changes - Move the blocking live-request receive into a narrow `wait_for_live_change/2` helper that only holds the change reference and timeout while parked. - Convert receive outcomes into small event values and handle response construction after the wait returns. - Keep garbage collection immediately before the parked receive, but run it inside the narrow helper so collection happens with a smaller root set. - Remove a dead `SystemMonitor` timer fallback branch that Elixir 1.20 reports as unreachable under warnings-as-errors. - Add changesets for sync-service and electric-telemetry. ## Decisions This keeps the waiter in the existing request process instead of introducing a central waiter registry. That avoids adding a new register/deregister bottleneck while still reducing the heap retained by parked live requests. The existing behavior is preserved for new changes, shape rotation, out-of-bounds checks, read-only timeout handling, and ordinary no-change responses.
1 parent 1b611ca commit b8be884

3 files changed

Lines changed: 92 additions & 60 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@core/sync-service': patch
3+
'@core/electric-telemetry': patch
4+
---
5+
6+
Reduce memory retained by live shape requests while they wait for changes.
7+
8+
Remove a dead SystemMonitor timer branch that fails warnings-as-errors on Elixir 1.20.

packages/electric-telemetry/lib/electric/telemetry/system_monitor.ex

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,4 @@ defmodule ElectricTelemetry.SystemMonitor do
170170
Process.send_after(self(), @garbage_collect_message, @garbage_collect_interval)
171171
}
172172
end
173-
174-
defp schedule_garbage_collect(state), do: state
175173
end

packages/sync-service/lib/electric/shapes/api.ex

Lines changed: 84 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -859,45 +859,97 @@ defmodule Electric.Shapes.Api do
859859
%{
860860
new_changes_ref: ref,
861861
handle: shape_handle,
862-
api: %{long_poll_timeout: long_poll_timeout} = api
862+
api: %{long_poll_timeout: long_poll_timeout}
863863
} = request
864864

865865
Logger.debug("Client #{inspect(self())} is waiting for changes to #{shape_handle}")
866866

867-
# Bandit reuses handler processes across requests. This process may have accumulated
868-
# garbage from previous requests or from building the response for this request.
869-
# Before blocking in receive (potentially for up to long_poll_timeout seconds),
870-
# run garbage collection so we don't hold onto memory while idle.
871-
# Combined with handler_fullsweep_after config, this helps keep handler memory usage low.
867+
ref
868+
|> wait_for_live_change(long_poll_timeout)
869+
|> handle_live_change_result(request)
870+
end
871+
872+
defp wait_for_live_change(ref, long_poll_timeout) do
873+
# Bandit reuses handler processes across requests. Entering this narrow helper before
874+
# collecting keeps the parked receive from retaining the larger request-handling frame.
872875
:erlang.garbage_collect()
873876

874877
receive do
875878
{^ref, :new_changes, latest_log_offset} ->
876-
# Stream new log since currently "held" offset
877-
%{request | last_offset: latest_log_offset}
879+
{:new_changes, latest_log_offset}
880+
881+
{^ref, :shape_rotation, new_handle} ->
882+
{:shape_rotation, new_handle}
883+
884+
{^ref, :shape_rotation} ->
885+
:shape_rotation
886+
887+
{^ref, :out_of_bounds_timeout} ->
888+
:out_of_bounds_timeout
889+
after
890+
long_poll_timeout ->
891+
:long_poll_timeout
892+
end
893+
end
894+
895+
defp handle_live_change_result({:new_changes, latest_log_offset}, request) do
896+
# Stream new log since currently "held" offset
897+
%{request | last_offset: latest_log_offset}
898+
|> determine_global_last_seen_lsn()
899+
|> determine_log_chunk_offset()
900+
|> determine_up_to_date()
901+
|> do_serve_shape_log()
902+
end
903+
904+
defp handle_live_change_result({:shape_rotation, new_handle}, request) do
905+
error = Api.Error.must_refetch()
906+
907+
Response.error(request, error.message,
908+
handle: new_handle,
909+
status: error.status
910+
)
911+
end
912+
913+
defp handle_live_change_result(:shape_rotation, request) do
914+
error = Api.Error.must_refetch()
915+
Response.error(request, error.message, status: error.status)
916+
end
917+
918+
defp handle_live_change_result(:out_of_bounds_timeout, request) do
919+
%{api: api, handle: shape_handle} = request
920+
921+
Logger.debug(fn ->
922+
"Client #{inspect(self())} timed out waiting for " <>
923+
"changes to #{shape_handle} (out-of-bounds check)"
924+
end)
925+
926+
case check_for_disk_updates(request) do
927+
{:updated, new_offset} ->
928+
%{request | last_offset: new_offset}
878929
|> determine_global_last_seen_lsn()
879930
|> determine_log_chunk_offset()
880931
|> determine_up_to_date()
881932
|> do_serve_shape_log()
882933

883-
{^ref, :shape_rotation, new_handle} ->
884-
error = Api.Error.must_refetch()
885-
886-
Response.error(request, error.message,
887-
handle: new_handle,
888-
status: error.status
889-
)
934+
_ ->
935+
Response.invalid_request(api, errors: @offset_out_of_bounds)
936+
end
937+
end
890938

891-
{^ref, :shape_rotation} ->
892-
error = Api.Error.must_refetch()
893-
Response.error(request, error.message, status: error.status)
939+
defp handle_live_change_result(:long_poll_timeout, request) do
940+
%{api: api} = request
941+
request = update_attrs(request, %{ot_is_long_poll_timeout: true})
942+
status = Electric.StatusMonitor.status(api.stack_id)
894943

895-
{^ref, :out_of_bounds_timeout} ->
896-
Logger.debug(fn ->
897-
"Client #{inspect(self())} timed out waiting for " <>
898-
"changes to #{shape_handle} (out-of-bounds check)"
899-
end)
944+
cond do
945+
request.read_only? or status.shape == :read_only ->
946+
# Align the request flag with the current runtime status so that
947+
# downstream functions (determine_log_chunk_offset, get_merged_log_stream,
948+
# etc.) use the correct read-only strategy.
949+
request = %{request | read_only?: true}
900950

951+
# No consumer is running (or it stopped), so check if the
952+
# active instance has flushed new data to disk.
901953
case check_for_disk_updates(request) do
902954
{:updated, new_offset} ->
903955
%{request | last_offset: new_offset}
@@ -907,45 +959,19 @@ defmodule Electric.Shapes.Api do
907959
|> do_serve_shape_log()
908960

909961
_ ->
910-
Response.invalid_request(api, errors: @offset_out_of_bounds)
911-
end
912-
after
913-
long_poll_timeout ->
914-
request = update_attrs(request, %{ot_is_long_poll_timeout: true})
915-
status = Electric.StatusMonitor.status(api.stack_id)
916-
917-
cond do
918-
request.read_only? or status.shape == :read_only ->
919-
# Align the request flag with the current runtime status so that
920-
# downstream functions (determine_log_chunk_offset, get_merged_log_stream,
921-
# etc.) use the correct read-only strategy.
922-
request = %{request | read_only?: true}
923-
924-
# No consumer is running (or it stopped), so check if the
925-
# active instance has flushed new data to disk.
926-
case check_for_disk_updates(request) do
927-
{:updated, new_offset} ->
928-
%{request | last_offset: new_offset}
929-
|> determine_global_last_seen_lsn()
930-
|> determine_log_chunk_offset()
931-
|> determine_up_to_date()
932-
|> do_serve_shape_log()
933-
934-
_ ->
935-
request
936-
|> determine_global_last_seen_lsn()
937-
|> no_change_response()
938-
end
939-
940-
status.shape == :up ->
941962
request
942963
|> determine_global_last_seen_lsn()
943964
|> no_change_response()
944-
945-
true ->
946-
message = Electric.StatusMonitor.timeout_message(api.stack_id)
947-
Response.error(request, message, status: 503, retry_after: 10)
948965
end
966+
967+
status.shape == :up ->
968+
request
969+
|> determine_global_last_seen_lsn()
970+
|> no_change_response()
971+
972+
true ->
973+
message = Electric.StatusMonitor.timeout_message(api.stack_id)
974+
Response.error(request, message, status: 503, retry_after: 10)
949975
end
950976
end
951977

0 commit comments

Comments
 (0)