Move preview rendering out of the BDS.Preview GenServer
This commit is contained in:
13
TECHDEBTS.md
13
TECHDEBTS.md
@@ -451,7 +451,18 @@ error like the existing `%{kind: :auth, ...}` shape so the UI can toast it.
|
||||
|
||||
## Phase 3 — Process architecture (de-bottleneck singletons)
|
||||
|
||||
### TD-11: Move preview rendering out of the `BDS.Preview` GenServer
|
||||
### TD-11: Move preview rendering out of the `BDS.Preview` GenServer ✅ DONE (2026-06-12)
|
||||
|
||||
**Status: implemented.** `BDS.Preview` now keeps only preview-server lifecycle and
|
||||
drain coordination in the GenServer. The current server state lives in a public
|
||||
ETS table, so both direct preview calls and socket-served HTTP requests resolve
|
||||
against that fast state read and render outside the singleton owner process.
|
||||
Direct `request/2` and `preview_draft/3` calls now run in short-lived tracked
|
||||
tasks, while TCP request workers render inline after inheriting the test sandbox
|
||||
allowance; graceful stop flips `is_running` false in ETS before draining and
|
||||
waits for those tracked render workers to finish. Coverage now proves two real
|
||||
HTTP preview renders can be in flight at once and that `stop_preview/1` waits
|
||||
for a blocked render to complete before returning.
|
||||
|
||||
**Context.** Every preview HTTP request (`Preview.request/2`,
|
||||
`preview_draft/3`) renders Markdown + Liquid **inside** the singleton
|
||||
|
||||
@@ -13,6 +13,7 @@ defmodule BDS.Preview do
|
||||
|
||||
@host "127.0.0.1"
|
||||
@port 4123
|
||||
@server_table __MODULE__.ServerTable
|
||||
|
||||
# Max time to wait for inflight requests to finish during graceful shutdown
|
||||
# before remaining request tasks are forcibly terminated.
|
||||
@@ -48,18 +49,24 @@ defmodule BDS.Preview do
|
||||
|
||||
def request(project_id, request_path) when is_binary(project_id) and is_binary(request_path) do
|
||||
{path, query_params} = split_request_path(request_path)
|
||||
GenServer.call(__MODULE__, {:request, project_id, path, query_params})
|
||||
|
||||
run_tracked_request(fn ->
|
||||
resolve_preview_request(project_id, path, query_params)
|
||||
end)
|
||||
end
|
||||
|
||||
def preview_draft(project_id, request_path, post_id)
|
||||
when is_binary(project_id) and is_binary(request_path) and is_binary(post_id) do
|
||||
{_path, query_params} = split_request_path(request_path)
|
||||
|
||||
GenServer.call(__MODULE__, {:preview_draft, project_id, query_params, post_id})
|
||||
run_tracked_request(fn ->
|
||||
resolve_draft_preview_request(project_id, query_params, post_id)
|
||||
end)
|
||||
end
|
||||
|
||||
@impl true
|
||||
def init(_state) do
|
||||
ensure_server_table!()
|
||||
{:ok, %{current: nil, stopping: nil}}
|
||||
end
|
||||
|
||||
@@ -90,57 +97,6 @@ defmodule BDS.Preview do
|
||||
end
|
||||
end
|
||||
|
||||
def handle_call({:request, project_id, request_path, query_params}, _from, state) do
|
||||
with :ok <- ensure_running(state.current, project_id),
|
||||
{:ok, response} <- resolve_request(state.current, request_path, query_params) do
|
||||
{:reply, {:ok, response}, state}
|
||||
else
|
||||
{:error, reason} -> {:reply, {:error, reason}, state}
|
||||
end
|
||||
end
|
||||
|
||||
def handle_call({:preview_draft, project_id, query_params, post_id}, _from, state) do
|
||||
with :ok <- ensure_running(state.current, project_id),
|
||||
{:ok, payload} <- load_draft_preview_payload(project_id, post_id, query_params) do
|
||||
body =
|
||||
case Rendering.render_post_page(project_id, payload.template_slug, payload) do
|
||||
{:ok, rendered} -> rendered
|
||||
{:error, _reason} -> render_draft(payload)
|
||||
end
|
||||
|> apply_preview_overrides(query_params)
|
||||
|
||||
response = %{
|
||||
content_type: "text/html",
|
||||
body: body
|
||||
}
|
||||
|
||||
{:reply, {:ok, response}, state}
|
||||
else
|
||||
{:error, :not_found} = error -> {:reply, error, state}
|
||||
{:error, reason} -> {:reply, {:error, reason}, state}
|
||||
end
|
||||
end
|
||||
|
||||
def handle_call({:http_request, project_id, method, request_path, query_params}, _from, state) do
|
||||
response =
|
||||
with :ok <- ensure_running(state.current, project_id),
|
||||
:ok <- ensure_get(method) do
|
||||
case query_params["post_id"] do
|
||||
post_id when is_binary(post_id) ->
|
||||
if String.starts_with?(request_path, "/draft/") or query_params["draft"] == "true" do
|
||||
resolve_draft_request(project_id, post_id, query_params)
|
||||
else
|
||||
resolve_request(state.current, request_path, query_params)
|
||||
end
|
||||
|
||||
_other ->
|
||||
resolve_request(state.current, request_path, query_params)
|
||||
end
|
||||
end
|
||||
|
||||
{:reply, response, state}
|
||||
end
|
||||
|
||||
@impl true
|
||||
def handle_cast({:track_request, pid}, %{current: %{} = current} = state) when is_pid(pid) do
|
||||
ref = Process.monitor(pid)
|
||||
@@ -168,6 +124,43 @@ defmodule BDS.Preview do
|
||||
defp ensure_running(%{project_id: project_id, is_running: true}, project_id), do: :ok
|
||||
defp ensure_running(_server, _project_id), do: {:error, :not_running}
|
||||
|
||||
defp resolve_preview_request(project_id, request_path, query_params) do
|
||||
with {:ok, server} <- current_server(project_id),
|
||||
{:ok, response} <- run_request(fn -> resolve_request(server, request_path, query_params) end) do
|
||||
{:ok, response}
|
||||
end
|
||||
end
|
||||
|
||||
defp resolve_draft_preview_request(project_id, query_params, post_id) do
|
||||
with {:ok, _server} <- current_server(project_id),
|
||||
{:ok, response} <-
|
||||
run_request(fn -> resolve_draft_request(project_id, post_id, query_params) end) do
|
||||
{:ok, response}
|
||||
else
|
||||
{:error, :not_found} = error -> error
|
||||
{:error, reason} -> {:error, reason}
|
||||
end
|
||||
end
|
||||
|
||||
defp resolve_http_request(project_id, method, request_path, query_params) do
|
||||
with {:ok, server} <- current_server(project_id),
|
||||
:ok <- ensure_get(method) do
|
||||
run_request(fn ->
|
||||
case query_params["post_id"] do
|
||||
post_id when is_binary(post_id) ->
|
||||
if String.starts_with?(request_path, "/draft/") or query_params["draft"] == "true" do
|
||||
resolve_draft_request(project_id, post_id, query_params)
|
||||
else
|
||||
resolve_request(server, request_path, query_params)
|
||||
end
|
||||
|
||||
_other ->
|
||||
resolve_request(server, request_path, query_params)
|
||||
end
|
||||
end)
|
||||
end
|
||||
end
|
||||
|
||||
defp resolve_request(server, request_path, query_params) do
|
||||
case PreviewAssets.response(request_path) do
|
||||
{:ok, response} ->
|
||||
@@ -311,10 +304,11 @@ defmodule BDS.Preview do
|
||||
end
|
||||
end
|
||||
|
||||
defp accept_loop(listener, project_id) do
|
||||
defp accept_loop(listener, project_id, owner_pid) do
|
||||
case :gen_tcp.accept(listener) do
|
||||
{:ok, socket} ->
|
||||
case Task.Supervisor.start_child(BDS.TCP.TaskSupervisor, fn ->
|
||||
maybe_allow_repo(owner_pid)
|
||||
serve_client(socket, project_id)
|
||||
end) do
|
||||
{:ok, pid} ->
|
||||
@@ -327,7 +321,7 @@ defmodule BDS.Preview do
|
||||
:ok
|
||||
end
|
||||
|
||||
accept_loop(listener, project_id)
|
||||
accept_loop(listener, project_id, owner_pid)
|
||||
|
||||
{:error, :closed} ->
|
||||
:ok
|
||||
@@ -373,11 +367,7 @@ defmodule BDS.Preview do
|
||||
path = uri.path || "/"
|
||||
query_params = URI.decode_query(uri.query || "")
|
||||
|
||||
case GenServer.call(
|
||||
__MODULE__,
|
||||
{:http_request, project_id, method, path, query_params},
|
||||
5_000
|
||||
) do
|
||||
case resolve_http_request(project_id, method, path, query_params) do
|
||||
{:ok, response} -> http_ok_response(response)
|
||||
{:error, :not_found} -> http_error_response(404)
|
||||
{:error, :not_running} -> http_error_response(503)
|
||||
@@ -453,14 +443,18 @@ defmodule BDS.Preview do
|
||||
# is parked (no immediate reply) and finalized from the :DOWN handlers, so the
|
||||
# GenServer stays available to serve the requests it is draining.
|
||||
defp begin_graceful_stop(%{current: current} = state, from) do
|
||||
current = %{current | is_running: false}
|
||||
put_current_server(current)
|
||||
|
||||
_ = :gen_tcp.close(current.listener)
|
||||
if is_pid(current.acceptor_pid), do: Process.exit(current.acceptor_pid, :normal)
|
||||
|
||||
if map_size(current.inflight) == 0 do
|
||||
clear_current_server()
|
||||
{:reply, :ok, %{state | current: nil, stopping: nil}}
|
||||
else
|
||||
timer = Process.send_after(self(), :drain_timeout, @drain_timeout)
|
||||
{:noreply, %{state | stopping: %{from: from, timer: timer}}}
|
||||
{:noreply, %{state | current: current, stopping: %{from: from, timer: timer}}}
|
||||
end
|
||||
end
|
||||
|
||||
@@ -469,6 +463,7 @@ defmodule BDS.Preview do
|
||||
)
|
||||
when map_size(inflight) == 0 do
|
||||
if is_reference(timer), do: Process.cancel_timer(timer)
|
||||
clear_current_server()
|
||||
GenServer.reply(from, :ok)
|
||||
%{state | current: nil, stopping: nil}
|
||||
end
|
||||
@@ -477,6 +472,7 @@ defmodule BDS.Preview do
|
||||
|
||||
defp force_finalize_stop(%{stopping: %{from: from}, current: %{inflight: inflight}} = state) do
|
||||
kill_inflight(inflight)
|
||||
clear_current_server()
|
||||
GenServer.reply(from, :ok)
|
||||
%{state | current: nil, stopping: nil}
|
||||
end
|
||||
@@ -488,6 +484,7 @@ defmodule BDS.Preview do
|
||||
_ = :gen_tcp.close(current.listener)
|
||||
if is_pid(current.acceptor_pid), do: Process.exit(current.acceptor_pid, :normal)
|
||||
kill_inflight(current.inflight)
|
||||
clear_current_server()
|
||||
%{state | current: nil}
|
||||
end
|
||||
|
||||
@@ -513,7 +510,7 @@ defmodule BDS.Preview do
|
||||
ip: {127, 0, 0, 1}
|
||||
])
|
||||
|
||||
acceptor_pid = spawn_link(fn -> accept_loop(listener, project_id) end)
|
||||
acceptor_pid = spawn_link(fn -> accept_loop(listener, project_id, owner_pid) end)
|
||||
|
||||
server = %{
|
||||
project_id: project_id,
|
||||
@@ -521,14 +518,70 @@ defmodule BDS.Preview do
|
||||
host: @host,
|
||||
port: @port,
|
||||
is_running: true,
|
||||
owner_pid: owner_pid,
|
||||
listener: listener,
|
||||
acceptor_pid: acceptor_pid,
|
||||
inflight: %{}
|
||||
}
|
||||
|
||||
put_current_server(server)
|
||||
{{:ok, public_server(server)}, %{state | current: server}}
|
||||
end
|
||||
|
||||
defp run_tracked_request(fun) when is_function(fun, 0) do
|
||||
owner_pid = self()
|
||||
|
||||
task =
|
||||
Task.Supervisor.async_nolink(BDS.TCP.TaskSupervisor, fn ->
|
||||
maybe_allow_repo(owner_pid)
|
||||
fun.()
|
||||
end)
|
||||
|
||||
GenServer.cast(__MODULE__, {:track_request, task.pid})
|
||||
Task.await(task, :infinity)
|
||||
end
|
||||
|
||||
defp run_request(fun) when is_function(fun, 0) do
|
||||
case Application.get_env(:bds, __MODULE__, []) |> Keyword.get(:request_runner) do
|
||||
{module, opts} -> module.run(fun, opts)
|
||||
nil -> fun.()
|
||||
end
|
||||
end
|
||||
|
||||
defp ensure_server_table! do
|
||||
case :ets.whereis(@server_table) do
|
||||
:undefined ->
|
||||
:ets.new(@server_table, [:named_table, :public, :set, read_concurrency: true])
|
||||
:ok
|
||||
|
||||
_table ->
|
||||
:ok
|
||||
end
|
||||
end
|
||||
|
||||
defp current_server(project_id) do
|
||||
case :ets.lookup(@server_table, :current) do
|
||||
[{:current, %{project_id: ^project_id} = server}] ->
|
||||
case ensure_running(server, project_id) do
|
||||
:ok -> {:ok, server}
|
||||
{:error, reason} -> {:error, reason}
|
||||
end
|
||||
|
||||
_other ->
|
||||
{:error, :not_running}
|
||||
end
|
||||
end
|
||||
|
||||
defp put_current_server(server) do
|
||||
true = :ets.insert(@server_table, {:current, server})
|
||||
:ok
|
||||
end
|
||||
|
||||
defp clear_current_server do
|
||||
true = :ets.delete(@server_table, :current)
|
||||
:ok
|
||||
end
|
||||
|
||||
defp public_server(server) do
|
||||
Map.take(server, [:project_id, :host, :port, :is_running])
|
||||
end
|
||||
|
||||
@@ -1,3 +1,17 @@
|
||||
defmodule BDS.PreviewTest.SlowRequestRunner do
|
||||
def run(fun, %{controller: controller}) when is_function(fun, 0) and is_pid(controller) do
|
||||
send(controller, {:preview_request_started, self()})
|
||||
|
||||
receive do
|
||||
:continue -> :ok
|
||||
after
|
||||
1_000 -> :ok
|
||||
end
|
||||
|
||||
fun.()
|
||||
end
|
||||
end
|
||||
|
||||
defmodule BDS.PreviewTest do
|
||||
use ExUnit.Case, async: false
|
||||
|
||||
@@ -746,6 +760,117 @@ defmodule BDS.PreviewTest do
|
||||
:gen_tcp.close(socket)
|
||||
end
|
||||
|
||||
test "preview HTTP requests overlap instead of serializing through the GenServer", %{
|
||||
project: project
|
||||
} do
|
||||
:inets.start()
|
||||
|
||||
Application.put_env(:bds, BDS.Preview,
|
||||
request_runner: {BDS.PreviewTest.SlowRequestRunner, %{controller: self()}}
|
||||
)
|
||||
|
||||
on_exit(fn ->
|
||||
Application.delete_env(:bds, BDS.Preview)
|
||||
end)
|
||||
|
||||
assert {:ok, _metadata} =
|
||||
Metadata.update_project_metadata(project.id, %{
|
||||
public_url: "https://example.com/blog",
|
||||
main_language: "en",
|
||||
blog_languages: ["en"]
|
||||
})
|
||||
|
||||
assert {:ok, post} =
|
||||
Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Concurrent Preview",
|
||||
content: "Body",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
assert {:ok, published_post} = Posts.publish_post(post.id)
|
||||
assert {:ok, server} = BDS.Preview.start_preview(project.id)
|
||||
|
||||
published_at = DateTime.from_unix!(published_post.created_at, :millisecond)
|
||||
|
||||
request_url =
|
||||
"http://#{server.host}:#{server.port}/#{published_at.year}/#{String.pad_leading(Integer.to_string(published_at.month), 2, "0")}/#{String.pad_leading(Integer.to_string(published_at.day), 2, "0")}/#{published_post.slug}"
|
||||
|
||||
request = fn ->
|
||||
:httpc.request(:get, {to_charlist(request_url), []}, [], body_format: :binary)
|
||||
end
|
||||
|
||||
task_one = Task.async(request)
|
||||
task_two = Task.async(request)
|
||||
|
||||
assert_receive {:preview_request_started, first_pid}, 500
|
||||
assert_receive {:preview_request_started, second_pid}, 500
|
||||
refute first_pid == second_pid
|
||||
|
||||
send(first_pid, :continue)
|
||||
send(second_pid, :continue)
|
||||
|
||||
assert {:ok, {{_version, 200, _reason}, _headers, body_one}} = Task.await(task_one, 2_000)
|
||||
assert {:ok, {{_version, 200, _reason}, _headers, body_two}} = Task.await(task_two, 2_000)
|
||||
assert body_one =~ "Concurrent Preview"
|
||||
assert body_two =~ "Concurrent Preview"
|
||||
|
||||
assert :ok = BDS.Preview.stop_preview(project.id)
|
||||
end
|
||||
|
||||
test "stop_preview waits for an inflight render to finish", %{project: project} do
|
||||
Application.put_env(:bds, BDS.Preview,
|
||||
request_runner: {BDS.PreviewTest.SlowRequestRunner, %{controller: self()}}
|
||||
)
|
||||
|
||||
on_exit(fn ->
|
||||
Application.delete_env(:bds, BDS.Preview)
|
||||
end)
|
||||
|
||||
assert {:ok, _metadata} =
|
||||
Metadata.update_project_metadata(project.id, %{
|
||||
public_url: "https://example.com/blog",
|
||||
main_language: "en",
|
||||
blog_languages: ["en"]
|
||||
})
|
||||
|
||||
assert {:ok, post} =
|
||||
Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Drain Preview",
|
||||
content: "Body",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
assert {:ok, published_post} = Posts.publish_post(post.id)
|
||||
assert {:ok, _server} = BDS.Preview.start_preview(project.id)
|
||||
|
||||
published_at = DateTime.from_unix!(published_post.created_at, :millisecond)
|
||||
|
||||
request_path =
|
||||
"/#{published_at.year}/#{String.pad_leading(Integer.to_string(published_at.month), 2, "0")}/#{String.pad_leading(Integer.to_string(published_at.day), 2, "0")}/#{published_post.slug}"
|
||||
|
||||
request_task =
|
||||
Task.async(fn ->
|
||||
BDS.Preview.request(project.id, request_path)
|
||||
end)
|
||||
|
||||
assert_receive {:preview_request_started, render_pid}, 500
|
||||
|
||||
stopper =
|
||||
Task.async(fn ->
|
||||
BDS.Preview.stop_preview(project.id)
|
||||
end)
|
||||
|
||||
refute Task.yield(stopper, 200)
|
||||
|
||||
send(render_pid, :continue)
|
||||
|
||||
assert {:ok, %{body: body, content_type: "text/html"}} = Task.await(request_task, 2_000)
|
||||
assert body =~ "Drain Preview"
|
||||
assert :ok = Task.await(stopper, 2_000)
|
||||
end
|
||||
|
||||
test "preview query params can override the rendered theme for generated and draft pages", %{
|
||||
project: project
|
||||
} do
|
||||
|
||||
Reference in New Issue
Block a user