diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 9254a61..a032986 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -60,6 +60,12 @@ "description": "Skills for writing and reviewing Product Requirements Documents (PRDs) and Feature Requirements Documents (FRDs)", "source": "./plugins/trogonstack-product-requirements", "category": "development" + }, + { + "name": "trogonstack-elixir", + "description": "Elixir skills for designing and reviewing OTP processes such as GenServers", + "source": "./plugins/trogonstack-elixir", + "category": "development" } ] } diff --git a/.github/release-please-config.json b/.github/release-please-config.json index 4e54971..32473e3 100644 --- a/.github/release-please-config.json +++ b/.github/release-please-config.json @@ -50,6 +50,9 @@ }, "plugins/trogonstack-product-requirements": { "component": "trogonstack-product-requirements" + }, + "plugins/trogonstack-elixir": { + "component": "trogonstack-elixir" } }, "plugins": [ diff --git a/.github/release-please-manifest.json b/.github/release-please-manifest.json index f18a564..e684f48 100644 --- a/.github/release-please-manifest.json +++ b/.github/release-please-manifest.json @@ -7,5 +7,6 @@ "plugins/trogonstack-otel": "0.1.2", "plugins/trogonstack-eventmodeling": "0.1.2", "plugins/trogonstack-eda": "0.1.2", - "plugins/trogonstack-product-requirements": "0.1.1" + "plugins/trogonstack-product-requirements": "0.1.1", + "plugins/trogonstack-elixir": "0.0.1" } diff --git a/plugins/trogonstack-elixir/.claude-plugin/plugin.json b/plugins/trogonstack-elixir/.claude-plugin/plugin.json new file mode 100644 index 0000000..a08d385 --- /dev/null +++ b/plugins/trogonstack-elixir/.claude-plugin/plugin.json @@ -0,0 +1,9 @@ +{ + "name": "trogonstack-elixir", + "description": "Elixir skills for designing and reviewing OTP processes such as GenServers", + "version": "0.0.1", + "author": { + "name": "TrogonStack", + "url": "https://github.com/TrogonStack" + } +} diff --git a/plugins/trogonstack-elixir/README.md b/plugins/trogonstack-elixir/README.md new file mode 100644 index 0000000..29b32c2 --- /dev/null +++ b/plugins/trogonstack-elixir/README.md @@ -0,0 +1,7 @@ +# trogonstack-elixir + +Elixir skills for designing and reviewing OTP processes such as GenServers. + +```bash +claude plugin install trogonstack-elixir@trogonstack +``` diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/SKILL.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/SKILL.md new file mode 100644 index 0000000..9e92ec3 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/SKILL.md @@ -0,0 +1,69 @@ +--- +name: elixir-design-genserver +description: "Design or review Elixir GenServer modules. Decides whether a process is justified at all, detects mailbox queuing and serialization bottlenecks, picks call vs cast, shapes the client API, and places the server under supervision. Use when writing a new GenServer, reviewing an existing one, deciding where an actor belongs, or diagnosing a process that became a bottleneck. Do not use for: (1) designing full supervision trees across an application, (2) distributed Erlang or clustering, (3) Phoenix LiveView or Channel processes, (4) GenStage, Broadway, or Flow pipelines." +allowed-tools: + - AskUserQuestion + - Read + - Write + - Shell +--- + +# Design an Elixir GenServer + +Put a process only where the runtime needs one, and know what it costs: every GenServer is a queue with a single worker. + +## Core Principle + +A process is the unit of concurrency, of serialization, and of failure, all at once. Use one to model a runtime concern (state shared across callers, an owned resource, an independent lifecycle, fault isolation). Never use one to organize code; modules and functions do that. + +Wrapping an operation that needs none of those concerns in a GenServer does not add structure. It adds a queue that turns concurrent callers into sequential ones. + +## Principles + +Apply them in order. A design that fails an earlier principle makes the later ones moot. Each one names the references that show it in practice; read them when the situation matches, not up front. + +### 1. A Process Must Own Something + +A process earns its place by owning what cannot live in the caller: shared state that must change consistently, a resource with its own lifecycle, time, or a failure boundary. If it owns nothing, the caller can do the work itself, concurrently. + +Read [references/process-justification.md](references/process-justification.md) when the decision is not obvious, or when reviewing code that uses GenServers as a service layer. + +Read [references/resource-lifecycle-ownership.md](references/resource-lifecycle-ownership.md) when a resource must be released when some process exits, or when cleanup depends on callers calling `release` or on `terminate/2`. + +Read [references/anti-pattern-database-gatekeeper.md](references/anti-pattern-database-gatekeeper.md) when a GenServer wraps database writes, or is justified as fault tolerance or as a way to avoid race conditions. + +### 2. Every Process Is a Queue + +One process handles one message at a time, so it caps throughput at `1 / handling_time` regardless of core count, and every caller waits behind every other. Know the arrival rate and handling time on the hot path before adding one, and remove the queue rather than tuning around it. + +Read [references/mailbox-queuing.md](references/mailbox-queuing.md) when the server sits on a request path, receives casts from many producers, does I/O inside callbacks, or already shows timeouts or a growing `message_queue_len`. It links to an avoid/prefer example for each way of removing the queue. + +### 3. The Process Is a Shell Around Meaning + +The domain lives in a pure, deterministic core named after what it means. The GenServer only translates messages into calls on that core and owns what the core must not know about: the clock, timers, monitors, and other processes. Logic that needs no process stays testable and reusable without one. + +Read [references/callback-patterns.md](references/callback-patterns.md) when writing or reviewing the module body. + +Read [references/handle-info.md](references/handle-info.md) when the server sends itself messages, uses timers, monitors other processes, or receives task results. + +Read [references/naming.md](references/naming.md) when naming the public module, the pure core, the process state, or the messages the GenServer exchanges. + +### 4. Contracts Outlive Implementations + +The client API is used by code you do not control and cannot change in lockstep. Shape what goes in and what comes out so it can grow without breaking callers. + +Read [references/request-response.md](references/request-response.md) when designing or changing what a client function takes or returns, especially a reply that holds a list. + +### 5. Crash Only When a Restart Helps + +A restart starts from a clean state, which fixes corrupt state and transient faults and nothing else. Match the restart strategy to the lifecycle, and handle failures that persist across restarts instead of crash-looping. + +Read [references/supervision-and-testing.md](references/supervision-and-testing.md) when wiring the child spec, handling cleanup, or writing tests. + +## Worked Example + +Read [references/example-chat-room.md](references/example-chat-room.md) to see every principle applied in one complete, tested GenServer. + +## Reviewing an Existing GenServer + +Read [references/review-checklist.md](references/review-checklist.md) when reviewing or auditing GenServer code. Each item is a symptom; a failing item is a question about the principle behind it, answered in the linked reference. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/anti-pattern-database-gatekeeper.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/anti-pattern-database-gatekeeper.md new file mode 100644 index 0000000..5b08279 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/anti-pattern-database-gatekeeper.md @@ -0,0 +1,103 @@ +# Anti-Pattern: Database Gatekeeper + +A GenServer stands in front of the database and every callback wraps a `Repo` call. It is usually built for fault tolerance or to serialize writes, and it delivers neither. The database already provides both, for every writer, on every node. + +## Avoid + +```elixir +defmodule MyApp.Accounts.AccountsServer do + use GenServer + + @impl GenServer + def init(_opts), do: {:ok, nil} + + @impl GenServer + def handle_call(%DebitRequest{account_id: account_id, amount: amount}, _from, state) do + account = Repo.get!(Account, account_id) + + reply = + account + |> Account.debit_changeset(amount) + |> Repo.update() + + {:reply, reply, state} + end +end +``` + +## What It Seems to Give + +| Belief | Reality | +|--------|---------| +| Supervision makes the write fault tolerant | A restart does not retry the write. The in-flight call is lost and the caller exits. The transaction is the failure boundary; retries belong to the caller, with an idempotency key. | +| One process linearizes writes, so there are no races | It serializes only callers that go through it, on one node. Other nodes, the overlap during a rolling deploy, migrations, jobs, other services, and `iex` all write around it. | +| It protects the database from load | The Repo pool already bounds connections and queues callers. The GenServer adds a pool of size one in front of it. | +| `cast` makes writes fast and async | The mailbox is not durable. A crash or deploy drops every queued write without a trace. | +| It caches rows in its state | Two sources of truth that drift the moment anyone else writes. | + +## Prefer + +Run the write in the caller's process and let the database linearize it: + +```elixir +defmodule MyApp.Accounts do + import Ecto.Query + alias Ecto.Multi + + def debit(account_id, amount) do + multi = + Multi.new() + |> Multi.run(:account, fn repo, _changes -> fetch_for_update(repo, account_id) end) + |> Multi.update(:debited, fn %{account: account} -> Account.debit_changeset(account, amount) end) + + case Repo.transact(multi) do + {:ok, %{debited: account}} -> {:ok, account} + {:error, _step, error, _changes} -> {:error, error} + end + end + + defp fetch_for_update(repo, account_id) do + query = Account |> where([a], a.id == ^account_id) |> lock("FOR UPDATE") + + case repo.one(query) do + nil -> {:error, %AccountNotFoundError{account_id: account_id}} + %Account{} = account -> {:ok, account} + end + end +end +``` + +The row lock serializes concurrent debits of the same account, from any process on any node, while debits of other accounts run in parallel. Each `Ecto.Multi` step runs in order inside one transaction; the first step that fails, a missing account or an invalid changeset, rolls the whole transaction back. `Repo.transact/2` (Ecto 3.13+) runs the multi and returns `{:error, step, error, changes}` on failure, which `debit/2` narrows to the [reply shapes](callback-patterns.md#reply-shapes). + +Pick the database mechanism that matches the guarantee: + +| Need | Use | +|------|-----| +| No duplicates | Unique index, and `Repo.insert(changeset, on_conflict: :nothing, conflict_target: :email)` for idempotent inserts, `on_conflict: {:replace, fields}` to update the existing row | +| No lost updates on a contended row | `lock("FOR UPDATE")` inside a transaction | +| No lost updates on a rarely contended row | `Ecto.Changeset.optimistic_lock/3`, retrying on `Ecto.StaleEntryError` | +| Ordering or sequence numbers | A database sequence or a version column | +| Mutual exclusion that is not a single row | `SELECT pg_advisory_xact_lock($1)` inside the transaction | +| Async writes that must survive a crash | A persisted job queue such as Oban | + +## When a Process Belongs Near the Database + +The process must own something the database does not: + +- Time and a buffer: write-behind batching that collects rows and flushes them with `Repo.insert_all/3` on a timer or size threshold. It trades durability for throughput, so document the loss window and flush in `terminate/2` with `Process.flag(:trap_exit, true)`. +- A connection-shaped resource: a `Postgrex.Notifications` listener for `LISTEN`/`NOTIFY`. +- Authoritative in-memory state: a game room or auction that runs in memory and persists snapshots. While it runs, the process is the source of truth and the database is the backup, never both at once. + +## Smells in Review + +- `handle_call/3` bodies are `Repo` calls and the state is `nil` or unused +- The process is justified as "fault tolerance" or "to avoid race conditions" +- Writes go through `cast` +- The state holds rows that other code also writes +- The module has no unique indexes, locks, or transactions because the process "already serializes" + +## Trade-offs + +- Row locks and advisory locks hold a connection while waiting; keep transactions short and never call external services inside them. +- Optimistic locking pushes retries to the caller; it fits rows that rarely conflict. +- Throughput cost of the gatekeeper is covered in [queuing-remove-the-process.md](queuing-remove-the-process.md). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/callback-patterns.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/callback-patterns.md new file mode 100644 index 0000000..f9fd110 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/callback-patterns.md @@ -0,0 +1,102 @@ +# Callback Patterns + +The GenServer module is plumbing between callers and the pure core. The patterns below keep that plumbing thin, give callers one predictable way to handle results, and keep the process safe to start and restart. + +## Module Layout + +```elixir +defmodule MyApp.RateLimiter do + use GenServer + + alias MyApp.RateLimiter.{Bucket, TakeRequest, TakeResponse} + + def start_link(opts) do + {name, opts} = Keyword.pop!(opts, :name) + GenServer.start_link(__MODULE__, opts, name: name) + end + + def take(server, %TakeRequest{} = request) do + GenServer.call(server, request) + end + + @impl GenServer + def init(opts) do + {:ok, Bucket.new(opts)} + end + + @impl GenServer + def handle_call(%TakeRequest{key: key}, _from, %Bucket{} = bucket) do + case Bucket.take(bucket, key) do + {:ok, bucket} -> + {:reply, {:ok, %TakeResponse{remaining: Bucket.remaining(bucket, key)}}, bucket} + + {:error, error} -> + {:reply, {:error, error}, bucket} + end + end +end +``` + +- Callers never call `GenServer.call/cast` directly. The client API owns the call; the messages are `Request` and `Response` structs. See [naming.md](naming.md). +- Callbacks delegate to a pure core (`Bucket.take/2`) that knows nothing about the GenServer or its messages. +- Mark every callback with `@impl GenServer`. +- Require `:name` in `start_link/1` and take the server as the first argument of every client function. Callers and tests always say which instance they talk to; a default argument hides that choice. A per-entity process derives its name from its key instead, and every client function takes that key first; see [example-chat-room.md](example-chat-room.md). +- Hold state in a struct so its shape is documented and enforced. The domain struct is the state until process-only data appears; see [naming.md](naming.md). + +## Reply Shapes + +Every reply, and every core function that can fail, returns one of: + +| Shape | When | +|-------|------| +| `:ok` | Success with nothing to return | +| `{:ok, value}` | Success with a value, usually an `Response` struct | +| `{:error, error}` | Failure. Any term works; an error struct (`defexception`) is recommended, see [naming.md](naming.md#errors) | + +Never reply with a tuple of more than two elements such as `{:ok, value, extra}` or `{:error, reason, details}`. Put the extra data in the struct. Callers can then handle every operation with the same `case` or `with`, and adding a field never breaks a pattern match. + +Core functions that cannot fail return their value directly, as `Bucket.remaining/2` returns an integer. When they return more than one value, they return a struct, never a loose tuple such as `{messages, more?}`. + +Reply with a Response struct rather than a bare list or value, so the reply can grow; see [request-response.md](request-response.md). + +This rule covers the replies you design. OTP callback return values such as `{:reply, reply, state}` or `{:ok, state, {:continue, term}}` keep the shapes OTP requires. + +## call vs cast vs send + +- `call`: the default. Back-pressure, errors reach the caller, the work is confirmed. +- `cast`: only when the caller does not care about the outcome and producers cannot outrun the server. See [mailbox-queuing.md](mailbox-queuing.md). +- `send/2` + `handle_info/2`: messages that are not part of the client API, such as timers, monitors, task results, and other libraries. See [handle-info.md](handle-info.md). + +## Initialization + +`init/1` blocks the caller of `start_link`; during boot that is the supervisor and every sibling after it. + +- Return `{:ok, state, {:continue, :load}}` and do slow work in `handle_continue/2`. +- The supervisor starts the next sibling as soon as `init/1` returns, before `handle_continue/2` finishes. A sibling that calls the server during its own startup waits in the mailbox behind the load and can time out. When siblings need the loaded state at boot, keep loading in `init/1`, or expose an explicit readiness signal (a `ready?/0` call, a `Registry` entry, a `:persistent_term` flag) that they check. +- Crash from `handle_continue/2` only when a restart can fix the failure. A dependency that stays down (database, remote API) crashes the server repeatedly, exhausts the supervisor's restart intensity (3 restarts in 5 seconds by default), and takes the supervisor and its other children down with it. For those failures, stay up in a degraded state and retry with `Process.send_after/3` and backoff. +- Return `:ignore` when configuration disables the server. + +## Timeouts and Long Work + +- `GenServer.call/3` defaults to 5 seconds and exits the caller on expiry. A slow call inside a serialized process is a design question before it is a timeout question. +- Offload slow work to a task and reply later with `GenServer.reply/2`; see [queuing-offload-slow-work.md](queuing-offload-slow-work.md). +- For periodic work, timers, monitors, and the `handle_info/2` catch-all, see [handle-info.md](handle-info.md). + +## Redacting State in Crash Logs + +Crash reports and `:sys.get_status/1` print the full state. When it holds secrets (tokens, credentials, personal data), implement `format_status/1` (Elixir 1.17+; earlier versions use the deprecated `format_status/2`) to redact them: + +```elixir +@impl GenServer +def format_status(%{state: %ClientServerState{} = state} = status) do + %{status | state: %ClientServerState{state | api_key: :redacted}} +end + +def format_status(status), do: status +``` + +## Process Names + +- Singleton: `name: __MODULE__`, only for genuinely one-per-node services. A singleton on a request path is a global queue. +- Many instances: `name: {:via, Registry, {MyApp.Registry, key}}` under a `DynamicSupervisor`. +- Never create atoms from user input to name processes. Atoms are not garbage collected. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/example-chat-room.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/example-chat-room.md new file mode 100644 index 0000000..0270f21 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/example-chat-room.md @@ -0,0 +1,470 @@ +# Example: Chat Room + +One complete GenServer with every principle applied. Read it to see how the pieces fit; read the linked references for why each piece looks the way it does. + +## Why a Process + +Each room owns the live conversation: the order of messages and who is in the room must change consistently while many members post at once. It also owns the subscriber processes it pushes messages to, and time, because an empty room stops itself. + +The room is ephemeral, like a live support chat: when it stops, its history goes with it. If history must survive, the database is the source of truth and reads go to the database, not through this process; see [anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md). + +Rooms are independent, so each one is its own process and a busy room never queues another; see [queuing-process-per-entity.md](queuing-process-per-entity.md). + +## Layout + +```text +MyApp.Chat public API, the only module callers use +MyApp.Chat.RoomServer one GenServer per room +MyApp.Chat.RoomServerState the room plus process-only data +MyApp.Chat.Room pure core: membership and bounded history +MyApp.Chat.Room.History core result for a page of messages +MyApp.Chat.Room.NotAMemberError core error +MyApp.Chat.Member domain struct +MyApp.Chat.Message domain struct +MyApp.Chat.*Request, *Response contract at the GenServer boundary +MyApp.Chat.MessagePosted event pushed to subscribers +``` + +```elixir +children = [ + {Registry, keys: :unique, name: MyApp.Chat.RoomRegistry}, + {DynamicSupervisor, name: MyApp.Chat.RoomSupervisor, strategy: :one_for_one} +] +``` + +## Pure Core + +```elixir +defmodule MyApp.Chat.Member do + defstruct [:id, :name] +end + +defmodule MyApp.Chat.Message do + defstruct [:id, :member_id, :body, :posted_at] +end + +defmodule MyApp.Chat.Room.History do + defstruct messages: [], more?: false +end + +defmodule MyApp.Chat.Room.NotAMemberError do + defexception [:member_id] + + @impl Exception + def message(%__MODULE__{member_id: member_id}), do: "#{inspect(member_id)} is not a member of the room" +end + +defmodule MyApp.Chat.Room do + alias MyApp.Chat.{Member, Message} + alias MyApp.Chat.Room.{History, NotAMemberError} + + defstruct [:id, :max_messages, members: %{}, messages: []] + + def new(id, max_messages), do: %__MODULE__{id: id, max_messages: max_messages} + + def join(%__MODULE__{} = room, %Member{} = member) do + %__MODULE__{room | members: Map.put(room.members, member.id, member)} + end + + def leave(%__MODULE__{} = room, member_id) do + %__MODULE__{room | members: Map.delete(room.members, member_id)} + end + + def post(%__MODULE__{} = room, %Message{} = message) do + if Map.has_key?(room.members, message.member_id) do + messages = Enum.take([message | room.messages], room.max_messages) + {:ok, %__MODULE__{room | messages: messages}} + else + {:error, %NotAMemberError{member_id: message.member_id}} + end + end + + def history(%__MODULE__{} = room, before, limit) do + older = if before, do: Enum.drop_while(room.messages, &(&1.id >= before)), else: room.messages + {page, rest} = Enum.split(older, limit) + %History{messages: page, more?: rest != []} + end +end +``` + +- No process, no clock, no ids: the server generates the message id and timestamp and passes a complete `Message` in. See [naming.md](naming.md#time-is-an-input). +- `join/2` and `leave/2` cannot fail, so they return the room. `post/2` can, so it returns `{:ok, room}` or `{:error, error}`. `history/3` returns a `History` struct, not a loose tuple. See [callback-patterns.md](callback-patterns.md#reply-shapes). +- History is bounded by `max_messages`, so the state and every reply stay small. See [queuing-large-replies.md](queuing-large-replies.md). + +## Contract + +```elixir +defmodule MyApp.Chat.JoinRequest do + @enforce_keys [:member] + defstruct [:member] +end + +defmodule MyApp.Chat.JoinResponse do + defstruct recent_messages: [], room_ref: nil +end + +defmodule MyApp.Chat.PostMessageRequest do + @enforce_keys [:member_id, :body] + defstruct [:member_id, :body] +end + +defmodule MyApp.Chat.PostMessageResponse do + defstruct [:message] +end + +defmodule MyApp.Chat.ListMessagesRequest do + defstruct limit: 50, before: nil +end + +defmodule MyApp.Chat.ListMessagesResponse do + defstruct messages: [], more?: false +end + +defmodule MyApp.Chat.MessagePosted do + defstruct [:room_id, :message] +end + +defmodule MyApp.Chat.RoomNotFoundError do + defexception [:room_id] + + @impl Exception + def message(%__MODULE__{room_id: room_id}), do: "room #{inspect(room_id)} is not running" +end +``` + +- Every operation takes a Request and answers with a Response, so each can grow by adding fields. See [request-response.md](request-response.md). +- `JoinResponse` carries the recent messages. Joining and reading the backlog in one call means no message can slip in between them. +- `@enforce_keys` is decided now, while there are no callers to break. + +## Server + +```elixir +defmodule MyApp.Chat.Subscriber do + defstruct [:member_id, :pid] +end + +defmodule MyApp.Chat.RoomServerState do + defstruct [:room, subscribers: %{}] +end + +defmodule MyApp.Chat.RoomServer do + use GenServer, restart: :temporary + + require Logger + + alias MyApp.Chat.{ + JoinRequest, + JoinResponse, + ListMessagesRequest, + ListMessagesResponse, + Message, + MessagePosted, + PostMessageRequest, + PostMessageResponse, + Room, + RoomNotFoundError, + RoomServerState, + Subscriber + } + + alias MyApp.Chat.Room.History + + @max_messages 500 + @backlog 50 + @idle_timeout :timer.minutes(30) + + def start_link(opts) do + room_id = Keyword.fetch!(opts, :room_id) + GenServer.start_link(__MODULE__, room_id, name: via(room_id)) + end + + def whereis(room_id) do + case Registry.lookup(MyApp.Chat.RoomRegistry, room_id) do + [{pid, _value}] -> {:ok, pid} + [] -> {:error, %RoomNotFoundError{room_id: room_id}} + end + end + + def ensure_started(room_id) do + with {:error, %RoomNotFoundError{}} <- whereis(room_id) do + case DynamicSupervisor.start_child(MyApp.Chat.RoomSupervisor, {__MODULE__, room_id: room_id}) do + {:ok, pid} -> {:ok, pid} + {:error, {:already_started, pid}} -> {:ok, pid} + {:error, error} -> {:error, error} + end + end + end + + defp via(room_id), do: {:via, Registry, {MyApp.Chat.RoomRegistry, room_id}} + + @impl GenServer + def init(room_id) do + {:ok, %RoomServerState{room: Room.new(room_id, @max_messages)}, @idle_timeout} + end + + @impl GenServer + def handle_call(%JoinRequest{member: member}, {pid, _tag}, %RoomServerState{} = state) do + ref = Process.monitor(pid) + subscribers = Map.put(state.subscribers, ref, %Subscriber{member_id: member.id, pid: pid}) + room = Room.join(state.room, member) + %History{} = history = Room.history(room, nil, @backlog) + reply(%RoomServerState{state | room: room, subscribers: subscribers}, {:ok, %JoinResponse{recent_messages: history.messages}}) + end + + def handle_call(%PostMessageRequest{} = request, _from, %RoomServerState{} = state) do + message = %Message{ + id: System.unique_integer([:positive, :monotonic]), + member_id: request.member_id, + body: request.body, + posted_at: DateTime.utc_now() + } + + case Room.post(state.room, message) do + {:ok, room} -> + broadcast(state, %MessagePosted{room_id: room.id, message: message}) + reply(%RoomServerState{state | room: room}, {:ok, %PostMessageResponse{message: message}}) + + {:error, error} -> + reply(state, {:error, error}) + end + end + + def handle_call(%ListMessagesRequest{} = request, _from, %RoomServerState{} = state) do + %History{} = history = Room.history(state.room, request.before, request.limit) + reply(state, {:ok, %ListMessagesResponse{messages: history.messages, more?: history.more?}}) + end + + @impl GenServer + def handle_info({:DOWN, ref, :process, _pid, _reason}, %RoomServerState{} = state) + when is_map_key(state.subscribers, ref) do + {%Subscriber{} = subscriber, subscribers} = Map.pop!(state.subscribers, ref) + still_present? = Enum.any?(Map.values(subscribers), &(&1.member_id == subscriber.member_id)) + room = if still_present?, do: state.room, else: Room.leave(state.room, subscriber.member_id) + noreply(%RoomServerState{state | room: room, subscribers: subscribers}) + end + + def handle_info(:timeout, %RoomServerState{subscribers: subscribers} = state) + when map_size(subscribers) == 0 do + {:stop, :normal, state} + end + + def handle_info(msg, %RoomServerState{} = state) do + Logger.warning("unexpected message: #{inspect(msg)}") + noreply(state) + end + + defp broadcast(%RoomServerState{} = state, %MessagePosted{} = event) do + Enum.each(Map.values(state.subscribers), &send(&1.pid, event)) + end + + defp reply(%RoomServerState{} = state, reply), do: {:reply, reply, state, idle_timeout(state)} + + defp noreply(%RoomServerState{} = state), do: {:noreply, state, idle_timeout(state)} + + defp idle_timeout(%RoomServerState{subscribers: subscribers}) when map_size(subscribers) == 0, + do: @idle_timeout + + defp idle_timeout(%RoomServerState{}), do: :infinity +end +``` + +- Callbacks translate messages into core calls and own only what the core must not know: subscriber pids, monitors, the clock, ids, and the idle timeout. That process-only data is why `RoomServerState` exists. See [naming.md](naming.md#process-state). +- The joining process is the subscriber, taken from `from`. The room monitors it and removes the member when its last subscriber goes down. See [handle-info.md](handle-info.md#monitors). +- `broadcast/2` uses `send/2`, which never blocks the room on a slow subscriber. +- An empty room returns `@idle_timeout` from every callback and stops with `:normal` when it fires. A room with subscribers returns `:infinity`. +- `restart: :temporary`: a restart would bring back an empty room that no subscriber is listening to, so a crashed room stays down and the next join starts a fresh one. +- `handle_info/2` ends with a catch-all. See [handle-info.md](handle-info.md#catch-all). + +## Public API + +```elixir +defmodule MyApp.Chat do + alias MyApp.Chat.{ + JoinRequest, + JoinResponse, + ListMessagesRequest, + PostMessageRequest, + RoomServer + } + + def join(room_id, %JoinRequest{} = request) do + with {:ok, pid} <- RoomServer.ensure_started(room_id) do + ref = Process.monitor(pid) + + case GenServer.call(pid, request) do + {:ok, %JoinResponse{} = response} -> + {:ok, %JoinResponse{response | room_ref: ref}} + + {:error, error} -> + Process.demonitor(ref, [:flush]) + {:error, error} + end + end + end + + def post_message(room_id, %PostMessageRequest{} = request), do: call(room_id, request) + + def list_messages(room_id, %ListMessagesRequest{} = request), do: call(room_id, request) + + defp call(room_id, request) do + with {:ok, pid} <- RoomServer.whereis(room_id) do + GenServer.call(pid, request) + end + end +end +``` + +- Callers use `MyApp.Chat` and never reference `RoomServer` or call `GenServer.call/2` themselves. +- Only `join/2` starts a room. Posting to or reading from a room that is not running returns `{:error, %RoomNotFoundError{}}` instead of creating an empty one. +- `whereis/1` checks the `Registry` first, so the `DynamicSupervisor` is called only when a room actually starts, not on every request. +- `join/2` monitors the room in the caller's process and returns the ref in `JoinResponse`. A subscriber that receives `:DOWN` for it knows the room is gone and can join again. + +## Tests + +```elixir +defmodule MyApp.Chat.RoomTest do + use ExUnit.Case, async: true + + alias MyApp.Chat.{Member, Message, Room} + alias MyApp.Chat.Room.{History, NotAMemberError} + + defp message(id, member_id), + do: %Message{id: id, member_id: member_id, body: "msg-#{id}", posted_at: DateTime.utc_now()} + + test "post by a non-member returns NotAMemberError" do + room = Room.new("room-1", 500) + + assert {:error, %NotAMemberError{member_id: 1}} = Room.post(room, message(1, 1)) + end + + test "history is bounded by max_messages" do + room = Room.new("room-1", 2) |> Room.join(%Member{id: 1, name: "alice"}) + + room = + Enum.reduce(1..3, room, fn id, room -> + {:ok, room} = Room.post(room, message(id, 1)) + room + end) + + assert %History{messages: messages} = Room.history(room, nil, 10) + assert Enum.map(messages, & &1.id) == [3, 2] + end + + test "history pages with before and reports more?" do + room = Room.new("room-1", 500) |> Room.join(%Member{id: 1, name: "alice"}) + + room = + Enum.reduce(1..5, room, fn id, room -> + {:ok, room} = Room.post(room, message(id, 1)) + room + end) + + assert %History{messages: page_1, more?: true} = Room.history(room, nil, 2) + assert Enum.map(page_1, & &1.id) == [5, 4] + + assert %History{messages: page_2, more?: true} = Room.history(room, 4, 2) + assert Enum.map(page_2, & &1.id) == [3, 2] + + assert %History{messages: page_3, more?: false} = Room.history(room, 2, 2) + assert Enum.map(page_3, & &1.id) == [1] + end +end +``` + +- The core needs no process, so every case builds a `Room` and calls it directly. See [supervision-and-testing.md](supervision-and-testing.md#testing). +- `before` takes the oldest id already seen and returns the next older page; walking the full history proves `more?` turns false only on the last page. + +```elixir +defmodule MyApp.Chat.RoomServerTest do + use ExUnit.Case, async: false + + alias MyApp.Chat + alias MyApp.Chat.{ + JoinRequest, + ListMessagesRequest, + Member, + MessagePosted, + PostMessageRequest, + PostMessageResponse, + RoomServer + } + + alias MyApp.Chat.Room.NotAMemberError + alias MyApp.Chat.RoomNotFoundError + + setup do + start_supervised!({Registry, keys: :unique, name: MyApp.Chat.RoomRegistry}) + start_supervised!({DynamicSupervisor, name: MyApp.Chat.RoomSupervisor, strategy: :one_for_one}) + :ok + end + + defp room_id, do: "room-#{System.unique_integer([:positive])}" + + test "join then post broadcasts MessagePosted to the subscriber" do + room_id = room_id() + member = %Member{id: 1, name: "alice"} + + assert {:ok, _} = Chat.join(room_id, %JoinRequest{member: member}) + + assert {:ok, %PostMessageResponse{message: message}} = + Chat.post_message(room_id, %PostMessageRequest{member_id: 1, body: "hi"}) + + assert_receive %MessagePosted{room_id: ^room_id, message: ^message} + end + + test "post and list on a room that is not running return RoomNotFoundError" do + room_id = room_id() + + assert {:error, %RoomNotFoundError{room_id: ^room_id}} = + Chat.post_message(room_id, %PostMessageRequest{member_id: 1, body: "hi"}) + + assert {:error, %RoomNotFoundError{room_id: ^room_id}} = + Chat.list_messages(room_id, %ListMessagesRequest{}) + end + + test "member is removed once its only subscriber process exits" do + room_id = room_id() + member = %Member{id: 1, name: "alice"} + test_pid = self() + + joiner = + spawn(fn -> + {:ok, _} = Chat.join(room_id, %JoinRequest{member: member}) + send(test_pid, :joined) + + receive do + :stop -> :ok + end + end) + + assert_receive :joined + ref = Process.monitor(joiner) + send(joiner, :stop) + assert_receive {:DOWN, ^ref, :process, ^joiner, :normal} + + assert {:error, %NotAMemberError{member_id: 1}} = + Chat.post_message(room_id, %PostMessageRequest{member_id: 1, body: "hi"}) + end + + test "an empty room stops normally on idle timeout" do + room_id = room_id() + {:ok, pid} = RoomServer.ensure_started(room_id) + + ref = Process.monitor(pid) + send(pid, :timeout) + + assert_receive {:DOWN, ^ref, :process, ^pid, :normal} + end +end +``` + +- The server is tested through `MyApp.Chat`, the same public API callers use, under the same children as [Layout](#layout). See [supervision-and-testing.md](supervision-and-testing.md#testing). +- `Registry` and `DynamicSupervisor` are started with the fixed names the example hardcodes, not a unique name per test, so tests running concurrently would share them; that forces `async: false`. Passing the names in, as [callback-patterns.md](callback-patterns.md) recommends for shared servers, is what keeps `async: true` possible. +- The exiting member is a separate spawned process, since the room monitors whoever called `join/2` and the test process itself must stay alive to assert. `assert_receive` on that process's own `:DOWN` confirms the exit before the next call runs. +- The idle-timeout test sends `:timeout` straight to the room instead of waiting `@idle_timeout` out; the room cannot tell a real timer from this message, so the assertion exercises the same `handle_info/2` clause either way. + +## Trade-offs + +- A caller can look up a room just before it stops idle and get an exit from `GenServer.call/2`. The room stops only when it has no subscribers, so active members never hit this. +- Subscribers receive `MessagePosted` in their mailbox. A subscriber that cannot keep up grows its own queue, not the room's; for fan-out across nodes, use `Phoenix.PubSub` instead of pids. +- One room is still one queue. A room with thousands of active posters needs its traffic measured; see [mailbox-queuing.md](mailbox-queuing.md). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/handle-info.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/handle-info.md new file mode 100644 index 0000000..fc74beb --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/handle-info.md @@ -0,0 +1,118 @@ +# handle_info and Self-Messaging + +Messages outside the client API are still a protocol. Give the ones you send a name, accept that you will receive ones you did not send, and never assume a message reflects the current state by the time it arrives. + +`handle_info/2` receives every message that did not come through `GenServer.call/cast`: messages the server sends itself, timers, monitors, task results, exit signals, and messages from libraries. + +## What Arrives Here + +| Message | Source | Shape | +|---------|--------|-------| +| Deferred work | `send(self(), msg)` | Your struct, for example `%ImportNextBatch{}` | +| Timer | `Process.send_after(self(), msg, ms)` | Your struct, for example `%RefillTick{}` | +| Monitored process died | `Process.monitor/1` | `{:DOWN, ref, :process, pid, reason}` | +| Task finished | `Task.Supervisor.async_nolink/2` | `{ref, result}`, then `:DOWN` unless flushed | +| Linked process exited | Only when trapping exits | `{:EXIT, pid, reason}` | +| Library message | PubSub, sockets, ports | Whatever the library defines | + +## Internal Message Structs + +Messages you send yourself are a private protocol. Define them as structs nested under the server module, like `Request` for client messages: + +```elixir +defmodule MyApp.RateLimiter.RefillTick do + defstruct [:token] +end +``` + +Match the tuples that OTP and libraries define (`:DOWN`, `{ref, result}`, `:EXIT`) as they are; you do not own those shapes. + +## Never Call Yourself + +```elixir +@impl GenServer +def handle_info(%RefillTick{}, %Bucket{} = bucket) do + RateLimiter.take(self(), %TakeRequest{key: :system}) + {:noreply, bucket} +end +``` + +`GenServer.call/3` to `self()` exits immediately with `:calling_self`. The indirect version is worse: server A calls B while B calls A, and both block until the timeout. Inside callbacks, call the pure core directly and never the module's own client API. Between servers, break call cycles with `cast`, `send`, or by moving the shared logic into a pure module both can use. + +## Deferring Work to Yourself + +`send(self(), msg)` and `{:continue, term}` both defer work, with opposite ordering: + +| Mechanism | Runs | Use for | +|-----------|------|---------| +| `{:continue, term}` | Immediately after the current callback, before any other message | Finishing setup or a follow-up step that must not interleave with clients | +| `send(self(), msg)` | After every message already in the mailbox | Yielding to clients between batches; see [queuing-chunk-long-work.md](queuing-chunk-long-work.md) | + +## Timers + +Use `Process.send_after/3` and reschedule from `handle_info/2`. Avoid `:timer.send_interval/2`, which keeps firing when the server falls behind and piles ticks into the mailbox. + +`Process.cancel_timer/1` cannot cancel a message that was already delivered. Tag each timer with a token and ignore ticks that do not match the current one: + +```elixir +@refill_interval :timer.seconds(1) + +defp schedule_refill(%RateLimiterServerState{} = state) do + if state.refill_timer, do: Process.cancel_timer(state.refill_timer) + token = make_ref() + timer = Process.send_after(self(), %RefillTick{token: token}, @refill_interval) + %RateLimiterServerState{state | refill_timer: timer, refill_token: token} +end + +@impl GenServer +def handle_info(%RefillTick{token: token}, %RateLimiterServerState{refill_token: token} = state) do + bucket = Bucket.refill(state.bucket, System.monotonic_time(:millisecond)) + {:noreply, schedule_refill(%RateLimiterServerState{state | bucket: bucket})} +end + +def handle_info(%RefillTick{}, %RateLimiterServerState{} = state) do + {:noreply, state} +end +``` + +- The server reads the clock and passes it to the core; `Bucket.refill/2` stays deterministic. See [naming.md](naming.md#time-is-an-input). +- Timer refs and tokens are process-only data, so they live in `RateLimiterServerState`, not in `Bucket`. + +## Monitors + +When state holds pids (subscribers, pending callers, workers), monitor them. Otherwise entries for dead processes stay in state forever. + +```elixir +@impl GenServer +def handle_call(%SubscribeRequest{pid: pid}, _from, %FeedServerState{} = state) do + ref = Process.monitor(pid) + {:reply, :ok, %FeedServerState{state | subscribers: Map.put(state.subscribers, ref, pid)}} +end + +@impl GenServer +def handle_info({:DOWN, ref, :process, _pid, _reason}, %FeedServerState{} = state) + when is_map_key(state.subscribers, ref) do + {:noreply, %FeedServerState{state | subscribers: Map.delete(state.subscribers, ref)}} +end +``` + +Monitor instead of link: a link makes the server die with the subscriber, a monitor only tells it. When the monitor exists to release a resource, see [resource-lifecycle-ownership.md](resource-lifecycle-ownership.md). + +## Task Results + +Results of `Task.Supervisor.async_nolink/2` arrive as `{ref, result}`, and a crash arrives as `:DOWN`. See [queuing-offload-slow-work.md](queuing-offload-slow-work.md) for the full pattern. + +## Catch-All + +`use GenServer` injects a `handle_info/2` that logs unexpected messages. Defining your own replaces it, so end with a catch-all clause or an unknown message crashes the server with a `FunctionClauseError`: + +```elixir +require Logger + +def handle_info(msg, state) do + Logger.warning("unexpected message: #{inspect(msg)}") + {:noreply, state} +end +``` + +Keep it as the last clause; clauses after it never match. `Logger.warning/1` is a macro, so the module needs `require Logger`. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/mailbox-queuing.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/mailbox-queuing.md new file mode 100644 index 0000000..631ad10 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/mailbox-queuing.md @@ -0,0 +1,89 @@ +# Mailbox Queuing + +A GenServer serializes everything sent to it. That is the point when it protects state that must change consistently, and pure cost when it does not. Remove serialization the problem does not need; bound the serialization it does. + +## The Model + +A GenServer is a queue with exactly one worker: + +- Messages land in an unbounded FIFO mailbox. +- The server handles one message at a time, to completion, before taking the next. +- Adding cores, schedulers, or callers does not add workers. + +Throughput ceiling: `1 / average_handling_time`. A callback that takes 10 ms caps the server at 100 messages per second on a 64-core machine. + +Latency under load (Little's law): a message waits roughly `queue_length * average_handling_time` before it is handled. When arrival rate exceeds the ceiling, the queue grows without bound and so does latency. + +## How Queuing Fails + +### With `call` + +- Each caller blocks until every message ahead of it is handled. +- The default 5 second timeout exits the caller, but the request stays in the mailbox and the server still does the work, then the reply is dropped. The call was tagged with an alias the caller removed on timeout, so the late reply never reaches its mailbox. Load does not drop when callers give up. +- Callers that retry on timeout add more messages to a queue that is already behind, so the backlog feeds itself. +- A server that calls another busy server inherits that server's queue as well. + +### With `cast` + +- No back-pressure: producers never slow down, so a slow server lets the mailbox grow until memory runs out. +- A large mailbox makes the process heap and garbage collection expensive, which slows handling further. +- Nothing tells the producer the server is behind; the first visible symptom is often memory alarms or the node going down. + +### Hidden Serialization + +The worst queues are the ones nobody meant to build: a GenServer that wraps work which was already concurrent (DB pool, HTTP client, pure functions). Each request is fast in isolation, tests pass, and production latency grows with traffic. + +## Detecting It + +```elixir +Process.info(pid, :message_queue_len) +``` + +- `:recon.proc_count(:message_queue_len, 10)` lists the processes with the longest mailboxes. +- `:observer.start()` Processes tab, sorted by MsgQ. + +Measure handling time per callback, so the `1 / handling_time` ceiling is a number rather than a guess: + +```elixir +@impl GenServer +def handle_call(%TakeRequest{key: key}, _from, %Bucket{} = bucket) do + result = + :telemetry.span([:my_app, :rate_limiter, :take], %{}, fn -> + {Bucket.take(bucket, key), %{}} + end) + + case result do + {:ok, bucket} -> {:reply, {:ok, %TakeResponse{remaining: Bucket.remaining(bucket, key)}}, bucket} + {:error, error} -> {:reply, {:error, error}, bucket} + end +end +``` + +Sample mailbox length with `telemetry_poller`: + +```elixir +{:telemetry_poller, + measurements: [ + {:process_info, name: MyApp.RateLimiter, event: [:my_app, :rate_limiter], keys: [:message_queue_len]} + ]} +``` + +Symptoms: `GenServer.call` timeouts that rise with load, latency that grows linearly with traffic, one scheduler pegged while others idle, process memory climbing on a single pid. + +## Patterns + +Each pattern has an avoid/prefer example. Pick the least invasive one that removes the queue. + +| Pattern | Read when | +|---------|-----------| +| [Remove the process](queuing-remove-the-process.md) | The server wraps a DB query, HTTP call, or pure computation and owns no state | +| [Serve reads from ETS](queuing-reads-from-ets.md) | Reads vastly outnumber writes and every read goes through the mailbox | +| [Offload slow work](queuing-offload-slow-work.md) | A callback blocks on I/O while other callers wait | +| [Chunk long work](queuing-chunk-long-work.md) | One message triggers a long job and client calls stall until it finishes | +| [Keep replies small](queuing-large-replies.md) | A `call` returns a large map or list | +| [Partition by key](queuing-partition-by-key.md) | Unrelated keys wait on each other in one server | +| [Process per entity](queuing-process-per-entity.md) | One server holds many independent entities (rooms, sessions, devices) | +| [Pool a resource](queuing-pool-resources.md) | The bottleneck is a scarce external resource, not state | +| [Shed load](queuing-shed-load.md) | The queue cannot be removed and callers must fail fast instead of timing out | + +Prefer `call` over `cast` from producers that can outrun the server: the blocked caller is the back-pressure. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/naming.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/naming.md new file mode 100644 index 0000000..39ea56e --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/naming.md @@ -0,0 +1,114 @@ +# Naming + +A GenServer usually involves a public API with process callbacks, the messages it exchanges, and a pure deterministic core that doubles as the process state. Name each one after what it means, not where it sits. + +## Recommended Layout + +```text +MyApp.RateLimiter public API and GenServer callbacks +MyApp.RateLimiter.TakeRequest message sent to the GenServer +MyApp.RateLimiter.TakeResponse value in the {:ok, response} reply +MyApp.RateLimiter.Bucket pure deterministic core, and the process state +MyApp.RateLimiter.Bucket.ExhaustedError error in the {:error, error} reply +``` + +```elixir +@impl GenServer +def handle_call(%TakeRequest{key: key}, _from, %Bucket{} = bucket) do + case Bucket.take(bucket, key) do + {:ok, bucket} -> + {:reply, {:ok, %TakeResponse{remaining: Bucket.remaining(bucket, key)}}, bucket} + + {:error, error} -> + {:reply, {:error, error}, bucket} + end +end +``` + +```elixir +defmodule MyApp.RateLimiter.Bucket do + alias MyApp.RateLimiter.Bucket.ExhaustedError + + defstruct [:capacity, tokens: %{}] + + def take(%__MODULE__{} = bucket, key) do + case remaining(bucket, key) do + 0 -> {:error, %ExhaustedError{key: key}} + n -> {:ok, %__MODULE__{bucket | tokens: Map.put(bucket.tokens, key, n - 1)}} + end + end + + def remaining(%__MODULE__{} = bucket, key) do + Map.get(bucket.tokens, key, bucket.capacity) + end +end +``` + +```elixir +defmodule MyApp.RateLimiter.Bucket.ExhaustedError do + defexception [:key] + + @impl Exception + def message(%__MODULE__{key: key}), do: "rate limit exhausted for #{inspect(key)}" +end +``` + +Replies follow the shapes in [callback-patterns.md](callback-patterns.md#reply-shapes): `:ok`, `{:ok, value}`, or `{:error, error}`. + +## Public Module + +Name it after the capability callers use: `MyApp.RateLimiter`. That it runs as a process is an implementation detail. + +When the callbacks outgrow a single module, move them to `MyApp.RateLimiter.RateLimiterServer` and keep the client API in `MyApp.RateLimiter`. Callers never reference the server module. + +Prefix the server module with the domain name instead of a bare `.Server`. A context may grow more than one server, and a bare `Server` collides across the codebase: every alias, stack trace, and search result reads `Server`, and aliasing two of them in one module forces `as:` renames. + +## Messages + +Exchange structs with the GenServer instead of tuples like `{:allow?, key}`. Name them `Request` and `Response`, nested under the public module: + +- The pairing stays unambiguous as operations grow: `TakeRequest` and `TakeResponse`, `RefillRequest` and `RefillResponse`. +- The client function takes the request and returns `{:ok, %TakeResponse{}}` or `{:error, error}`. + +For why bare lists and values never appear in replies, and how to grow these structs without breaking callers, see [request-response.md](request-response.md). + +Messages the server sends itself (`send/2`, timers) are structs too, for example `%RefillTick{}`; see [handle-info.md](handle-info.md#internal-message-structs). + +Request and Response exist only at the GenServer boundary. The pure core never sees them; `handle_call/3` unpacks the request, calls the core with domain values, and builds the response. + +## Pure Core + +Name it after the domain concept: `Bucket`, `Room`, `Session`, `Ledger`. The struct and the functions that operate on it live in the same module, and they take and return that struct. + +Avoid role names: `State`, `Core`, `Logic`, `Impl`, `Engine`, `Worker`, `Manager`, `Service`. They describe architecture rather than meaning, repeat in every GenServer, and tie the core to the process even when it is used without one. + +Name core functions with domain verbs (`take`, `refill`, `expire`), never GenServer vocabulary (`handle_*`). + +## Errors + +Any term can be the error in `{:error, error}`, and an atom such as `:not_found` is common. Prefer `defexception` structs named `*Error`, nested under the module that produces them: `Bucket.ExhaustedError`. An exception struct in `{:error, error}` is not raised; it gains `Exception.message/1` for logs and lets a bang variant `raise` the same value. Build the message from structured fields so callers can still match on them. An atom has no room for detail, so the first field it needs replaces the shape, the same problem [request-response.md](request-response.md#growing-without-breaking) describes for replies. + +## Time Is an Input + +The core never reads the clock. The server reads it and passes it in, so the same inputs always produce the same output and tests need no sleeping or mocking: + +```elixir +Bucket.refill(bucket, System.monotonic_time(:millisecond)) +``` + +The same applies to randomness and ids: generate them in the server, pass them to the core. + +## Process State + +Start with the domain struct as the process state: `init/1` returns `{:ok, %Bucket{}}`. Add a wrapper only when the process holds something the domain should not know about. + +| Situation | State | +|-----------|-------| +| Only domain data | The domain struct: `%Bucket{}` | +| Process-only data too: timer refs, monitor refs, pending `from`s, task refs | `%RateLimiterServerState{bucket: %Bucket{}, refill_timer: ref}` | +| Several domain structs | `%RateLimiterServerState{bucket: %Bucket{}, quota: %Quota{}}` | +| No domain logic, only bookkeeping | `%RateLimiterServerState{}` and no core module | + +Process-only data stays out of the domain struct because refs and `from`s are runtime plumbing, not deterministic data; putting them in the core makes it harder to test and reuse without a process. + +Name the wrapper `ServerState`, for example `MyApp.RateLimiter.RateLimiterServerState`, for the same reason the server module carries the domain name. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/process-justification.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/process-justification.md new file mode 100644 index 0000000..e3525cb --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/process-justification.md @@ -0,0 +1,51 @@ +# Where an Actor Belongs, and Why + +A process is justified by what it owns at runtime, never by the code it contains. Code is organized by modules; a process exists because something must be owned by exactly one place while many callers use it. + +## The Question to Ask + +"What does this process own that cannot live in the caller?" + +Valid answers: + +| Owns | Example | +|------|---------| +| Mutable state shared by many callers that must change consistently | Rate limiter buckets, in-memory counters, a game room | +| A resource with its own lifecycle | Socket, port, file handle, external connection | +| Time | Periodic jobs, debouncing, expiring sessions | +| A failure boundary | Code that may crash and must not take the caller down | + +Invalid answers: + +- "It is the service layer for users / orders / billing." That is a module. +- "It encapsulates the logic." Modules encapsulate logic. +- "It wraps the database / HTTP client." Those calls are already concurrent and pooled; a GenServer in front serializes them. +- "It makes writes fault tolerant" or "it prevents race conditions." Neither holds for database writes; see [anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md). +- "It runs in the background." That is a `Task` under a `Task.Supervisor`. +- "It caches data." Reads belong in ETS or `:persistent_term`; a process may own the table, but readers should not message it. + +## Picking the Right Tool + +| Need | Use | +|------|-----| +| Pure transformation of data | Plain module and functions | +| DB query or HTTP request | Call it from the caller's process | +| One-off concurrent work, result awaited | `Task` / `Task.async_stream` | +| Fire-and-forget work that must survive the caller | `Task.Supervisor.start_child` | +| Read-heavy shared data | ETS table owned by a process ([queuing-reads-from-ets.md](queuing-reads-from-ets.md)), or `:persistent_term` for rarely changing data | +| Shared state with logic, lifecycle, or timers | `GenServer` | +| One process per entity, looked up by key | `GenServer` + `Registry` + `DynamicSupervisor` ([queuing-process-per-entity.md](queuing-process-per-entity.md)) | +| Keyed state bottlenecked in one server | `PartitionSupervisor` (Elixir 1.14+, [queuing-partition-by-key.md](queuing-partition-by-key.md)) | + +## The Anti-Pattern + +A GenServer whose state is never used, forwarding every request to the database or an HTTP client. See [queuing-remove-the-process.md](queuing-remove-the-process.md) for the example and its fix. + +## Smells in Review + +- `state` is ignored or is `nil` / `%{}` forever +- Every `handle_call` clause is a thin pass-through to another module +- Callbacks wrap `Repo` calls ([anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md)) +- The module is a named singleton and every web request calls it +- The GenServer exists "for consistency" with other modules +- Removing `use GenServer` and the callbacks would change nothing but concurrency diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-chunk-long-work.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-chunk-long-work.md new file mode 100644 index 0000000..1bf9a2a --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-chunk-long-work.md @@ -0,0 +1,51 @@ +# Chunk Long Work + +A single message that triggers a long job holds the server for the whole job. Split the job into batches and send the next batch to yourself, so client messages that arrive in between are handled between batches. + +If the job does not need server state, run it in a `Task` instead; see [queuing-offload-slow-work.md](queuing-offload-slow-work.md). + +## Avoid + +```elixir +@impl GenServer +def handle_call(%ImportRequest{rows: rows}, _from, %Catalog{} = catalog) do + catalog = Enum.reduce(rows, catalog, &Catalog.import_row(&2, &1)) + {:reply, :ok, catalog} +end +``` + +Importing 100,000 rows blocks every lookup until the import ends. + +## Prefer + +```elixir +@batch_size 500 + +@impl GenServer +def handle_call(%ImportRequest{rows: rows}, _from, %CatalogServerState{} = state) do + if :queue.is_empty(state.import_queue), do: send(self(), %ImportNextBatch{}) + import_queue = Enum.reduce(rows, state.import_queue, &:queue.in/2) + response = %ImportResponse{pending_rows: :queue.len(import_queue)} + {:reply, {:ok, response}, %CatalogServerState{state | import_queue: import_queue}} +end + +@impl GenServer +def handle_info(%ImportNextBatch{}, %CatalogServerState{} = state) do + {batch, rest} = :queue.split(min(@batch_size, :queue.len(state.import_queue)), state.import_queue) + catalog = Enum.reduce(:queue.to_list(batch), state.catalog, &Catalog.import_row(&2, &1)) + if :queue.len(rest) > 0, do: send(self(), %ImportNextBatch{}) + {:noreply, %CatalogServerState{state | catalog: catalog, import_queue: rest}} +end +``` + +- `send(self(), msg)` puts the message at the back of the mailbox, behind client messages already waiting. That is what lets them interleave. +- `{:continue, term}` would run before any other message, which defeats the purpose here; see [handle-info.md](handle-info.md#deferring-work-to-yourself). +- `import_queue` starts as `:queue.new()`. Appending with `++` copies the whole backlog on every import; `:queue` appends and splits without that cost. +- A marker is pending exactly when the queue is not empty: a request that finds the queue empty schedules the first batch, and a batch schedules the next one only if rows remain. A second import joins the queue; a second marker would start a second chain and double the batch rate. +- The reply means "accepted", not "imported", and reports the backlog. Progress is new fields on `ImportResponse` or a separate request. + +## Trade-offs + +- Lookups during the import see a partially imported catalog. If that is not acceptable, build the new catalog in a task and swap it in with one message. +- Batch size trades import throughput against lookup latency: a lookup waits at most one batch. +- The rows are copied into the server with the request; see [queuing-large-replies.md](queuing-large-replies.md). For large imports, send a source such as a file path and read it batch by batch. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-large-replies.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-large-replies.md new file mode 100644 index 0000000..4781020 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-large-replies.md @@ -0,0 +1,42 @@ +# Keep Replies Small + +Every message, including every reply, is copied from one process heap to the other. A `call` that returns a large map or list makes the server spend its time copying while the queue grows, and spikes memory in the caller. + +Binaries larger than 64 bytes are the exception: they live in a shared heap and only a reference is copied. + +## Avoid + +```elixir +@impl GenServer +def handle_call(%GetRoomRequest{}, _from, %Room{} = room) do + {:reply, {:ok, %GetRoomResponse{room: room}}, room} +end +``` + +```elixir +{:ok, %GetRoomResponse{room: room}} = RoomServer.get_room(room_id, %GetRoomRequest{}) +Enum.find(room.members, &(&1.user_id == user_id)) +``` + +The caller wants one member and receives the whole room: every member and the full message history. Handing out the state is how this usually starts. + +## Prefer + +Ask the process the question instead of asking for its state: + +```elixir +@impl GenServer +def handle_call(%FetchMemberRequest{user_id: user_id}, _from, %Room{} = room) do + case Room.fetch_member(room, user_id) do + {:ok, member} -> {:reply, {:ok, %FetchMemberResponse{member: member}}, room} + {:error, error} -> {:reply, {:error, error}, room} + end +end +``` + +When many callers read the same data, serve it from ETS instead of the mailbox; see [queuing-reads-from-ets.md](queuing-reads-from-ets.md). + +## Trade-offs + +- Filtering in the server moves CPU into the serialized process. Keep the filter cheap, an indexed lookup, not a scan. +- The server state itself is never copied by callbacks; only messages are. A large state is a memory concern, not a copying one. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-offload-slow-work.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-offload-slow-work.md new file mode 100644 index 0000000..abdd8b7 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-offload-slow-work.md @@ -0,0 +1,95 @@ +# Offload Slow Work + +A callback that blocks on I/O stalls every caller behind it. Start the work in a task, return immediately, and reply when the task finishes. + +First ask whether the server is needed at all. If the work does not read or change server state, the caller can run it; see [queuing-remove-the-process.md](queuing-remove-the-process.md). This pattern is for servers that must coordinate the work, for example to deduplicate concurrent fetches or to update state with the result. + +## Avoid + +```elixir +@impl GenServer +def handle_call(%FetchRateRequest{currency: currency}, _from, %RateCache{} = cache) do + case RatesClient.fetch(currency) do + {:ok, rate} -> {:reply, {:ok, %FetchRateResponse{rate: rate}}, RateCache.put(cache, currency, rate)} + {:error, error} -> {:reply, {:error, error}, cache} + end +end +``` + +A 2 second HTTP call caps the server at one request every 2 seconds, and every other caller times out. + +## Prefer + +```elixir +@impl GenServer +def handle_call(%FetchRateRequest{currency: currency}, from, %RatesServerState{} = state) do + case RateCache.fetch(state.cache, currency) do + {:ok, rate} -> {:reply, {:ok, %FetchRateResponse{rate: rate}}, state} + {:error, %RateNotCachedError{}} -> {:noreply, await_fetch(state, currency, from)} + end +end + +@impl GenServer +def handle_info({ref, result}, %RatesServerState{} = state) when is_map_key(state.fetching, ref) do + Process.demonitor(ref, [:flush]) + {%PendingFetch{} = fetch, state} = pop_fetch(state, ref) + + case result do + {:ok, rate} -> + reply_all(fetch, {:ok, %FetchRateResponse{rate: rate}}) + {:noreply, %RatesServerState{state | cache: RateCache.put(state.cache, fetch.currency, rate)}} + + {:error, error} -> + reply_all(fetch, {:error, error}) + {:noreply, state} + end +end + +def handle_info({:DOWN, ref, :process, _pid, reason}, %RatesServerState{} = state) + when is_map_key(state.fetching, ref) do + {%PendingFetch{} = fetch, state} = pop_fetch(state, ref) + reply_all(fetch, {:error, %FetchFailedError{reason: reason}}) + {:noreply, state} +end + +defp await_fetch(%RatesServerState{} = state, currency, from) do + case Map.fetch(state.pending, currency) do + {:ok, %PendingFetch{} = fetch} -> + pending = Map.put(state.pending, currency, %PendingFetch{fetch | callers: [from | fetch.callers]}) + %RatesServerState{state | pending: pending} + + :error -> + task = Task.Supervisor.async_nolink(state.task_supervisor, RatesClient, :fetch, [currency]) + fetch = %PendingFetch{currency: currency, callers: [from]} + + %RatesServerState{ + state + | pending: Map.put(state.pending, currency, fetch), + fetching: Map.put(state.fetching, task.ref, currency) + } + end +end + +defp pop_fetch(%RatesServerState{} = state, ref) do + {currency, fetching} = Map.pop!(state.fetching, ref) + {fetch, pending} = Map.pop!(state.pending, currency) + {fetch, %RatesServerState{state | pending: pending, fetching: fetching}} +end + +defp reply_all(%PendingFetch{callers: callers}, reply) do + Enum.each(callers, &GenServer.reply(&1, reply)) +end +``` + +- What the process owns is coordination: a cache hit replies at once, and concurrent misses for the same currency share one fetch instead of each calling the API. +- `async_nolink` keeps a crashing task from taking the server down; the crash arrives as `:DOWN` and every waiting caller gets an error. +- Returning `{:noreply, state}` from `handle_call/3` frees the server; `GenServer.reply/2` answers the callers later. +- `Process.demonitor(ref, [:flush])` drops the `:DOWN` message that would otherwise follow a successful result. +- Pending callers and task refs are process-only data, so they live in the `RatesServerState` wrapper and not in `RateCache`; see [naming.md](naming.md#process-state). + +## Trade-offs + +- The caller still waits for the I/O, but other callers no longer wait for it. +- Cache hits still go through the mailbox. When they dominate, serve them from ETS as well; see [queuing-reads-from-ets.md](queuing-reads-from-ets.md). +- The caller's `call` timeout still applies. A reply after the timeout is discarded. +- Keep the clauses above the `handle_info/2` catch-all; see [handle-info.md](handle-info.md). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-partition-by-key.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-partition-by-key.md new file mode 100644 index 0000000..2075afa --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-partition-by-key.md @@ -0,0 +1,61 @@ +# Partition by Key + +When requests for unrelated keys wait on each other in one server, run several copies and route each key to the same copy. Keys still serialize against themselves, which preserves per-key consistency, but no longer against each other. + +## Avoid + +```elixir +children = [ + {MyApp.RateLimiter, name: MyApp.RateLimiter, capacity: 100} +] + +MyApp.RateLimiter.take(MyApp.RateLimiter, %TakeRequest{key: key}) +``` + +Every take for every key goes through one mailbox. + +## Prefer + +```elixir +children = [ + {PartitionSupervisor, child_spec: {MyApp.RateLimiter, capacity: 100}, name: MyApp.RateLimiterPartitions} +] +``` + +```elixir +defmodule MyApp.RateLimiter do + use GenServer + + alias MyApp.RateLimiter.{Bucket, TakeRequest, TakeResponse} + + def start_link(opts) do + GenServer.start_link(__MODULE__, opts) + end + + def take(partitions, %TakeRequest{key: key} = request) do + GenServer.call({:via, PartitionSupervisor, {partitions, key}}, request) + end + + @impl GenServer + def init(opts), do: {:ok, Bucket.new(opts)} + + @impl GenServer + def handle_call(%TakeRequest{key: key}, _from, %Bucket{} = bucket) do + case Bucket.take(bucket, key) do + {:ok, bucket} -> {:reply, {:ok, %TakeResponse{remaining: Bucket.remaining(bucket, key)}}, bucket} + {:error, error} -> {:reply, {:error, error}, bucket} + end + end +end +``` + +- `PartitionSupervisor` starts one child per scheduler by default (`partitions: System.schedulers_online()`). +- Partitions are not named. Callers address them through the `PartitionSupervisor` with the routing key, so `start_link/1` takes no `:name`. +- The same key always routes to the same partition, so each key keeps its own bucket. + +## Trade-offs + +- Partition only when per-key logic needs a process. A plain counter needs none: `:counters` or `:ets.update_counter/3` updates it from the caller without a mailbox. +- Anything that spans keys (totals, "list all") must ask every partition; `PartitionSupervisor.which_children/1` lists them. +- A hot key still serializes on its partition. If one key dominates, partitioning does not help; see [queuing-process-per-entity.md](queuing-process-per-entity.md) or [queuing-reads-from-ets.md](queuing-reads-from-ets.md). +- A crashed partition loses only its own keys' state. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-pool-resources.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-pool-resources.md new file mode 100644 index 0000000..96daf39 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-pool-resources.md @@ -0,0 +1,46 @@ +# Pool a Resource + +When the bottleneck is a scarce external resource (connections, sockets, licenses), the fix is a pool of them, not one process guarding a single instance. + +## Avoid + +```elixir +@impl GenServer +def init(opts) do + {:ok, conn} = PaymentsClient.connect(opts) + {:ok, conn} +end + +@impl GenServer +def handle_call(%ChargeRequest{charge: charge}, _from, conn) do + {:reply, PaymentsClient.charge(conn, charge), conn} +end +``` + +One connection, one request at a time, for the whole node. + +## Prefer + +Use the pool your client library already ships before writing one. Ecto pools database connections and Finch pools HTTP connections: + +```elixir +children = [ + {Finch, name: MyApp.Finch, pools: %{"https://payments.example.com" => [size: 25]}} +] + +Finch.build(:post, "https://payments.example.com/charges", headers, body) +|> Finch.request(MyApp.Finch) +``` + +When no library pool exists, build one with `NimblePool`, which checks a resource out to the caller's process so the work runs there and the pool process only hands out resources: + +```elixir +NimblePool.checkout!(MyApp.PaymentsPool, :checkout, fn _from, conn -> + {PaymentsClient.charge(conn, charge), conn} +end) +``` + +## Trade-offs + +- Pool size is a capacity decision: too small queues callers at checkout, too large overwhelms the external system. +- Checkout has its own timeout; treat a checkout timeout as overload, not as a bug. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-process-per-entity.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-process-per-entity.md new file mode 100644 index 0000000..24f61fa --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-process-per-entity.md @@ -0,0 +1,75 @@ +# Process per Entity + +When one server holds many independent entities, each with its own state and lifecycle, give each entity its own process. Activity in one room no longer queues behind activity in every other room, and a crash affects one entity. + +## Avoid + +```elixir +@impl GenServer +def handle_call(%PostMessageRequest{room_id: room_id, message: message}, _from, %Rooms{} = rooms) do + case Rooms.post(rooms, room_id, message) do + {:ok, rooms} -> {:reply, :ok, rooms} + {:error, error} -> {:reply, {:error, error}, rooms} + end +end +``` + +One process holds every chat room; a busy room slows down all of them. + +## Prefer + +```elixir +children = [ + {Registry, keys: :unique, name: MyApp.Chat.RoomRegistry}, + {DynamicSupervisor, name: MyApp.Chat.RoomSupervisor, strategy: :one_for_one} +] +``` + +```elixir +defmodule MyApp.Chat.RoomServer do + use GenServer, restart: :temporary + + alias MyApp.Chat.{PostMessageRequest, Room} + + def start_link(opts) do + room_id = Keyword.fetch!(opts, :room_id) + GenServer.start_link(__MODULE__, room_id, name: via(room_id)) + end + + def ensure_started(room_id) do + case DynamicSupervisor.start_child(MyApp.Chat.RoomSupervisor, {__MODULE__, room_id: room_id}) do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + {:error, error} -> {:error, error} + end + end + + def post(room_id, %PostMessageRequest{} = request) do + GenServer.call(via(room_id), request) + end + + defp via(room_id), do: {:via, Registry, {MyApp.Chat.RoomRegistry, room_id}} + + @impl GenServer + def init(room_id), do: {:ok, Room.new(room_id)} + + @impl GenServer + def handle_call(%PostMessageRequest{message: message}, _from, %Room{} = room) do + case Room.post(room, message) do + {:ok, room} -> {:reply, :ok, room} + {:error, error} -> {:reply, {:error, error}, room} + end + end +end +``` + +- `Registry` maps the entity id to its pid, so callers address rooms by id. +- `:temporary`: the room lives only in memory, so a restart would bring back an empty room. A crashed room stays down and the next `ensure_started/1` starts a fresh one. +- `ensure_started/1` treats `:already_started` as success, which covers two callers racing to start the same room: the `:unique` `Registry` key makes the second registration fail atomically. +- For the full room with its core, contract, monitors, and idle stop, see [example-chat-room.md](example-chat-room.md). + +## Trade-offs + +- Idle entities hold memory. Stop them after inactivity, for example by returning a timeout from callbacks and stopping with `{:stop, :normal, state}` on `:timeout`. +- State is lost when the process stops. Load it from durable storage in `handle_continue/2` if it must survive; a restart then recovers real state, so `:transient` fits. +- Cross-entity operations become messages between processes; keep them rare or move them to a separate read model. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-reads-from-ets.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-reads-from-ets.md new file mode 100644 index 0000000..7d7f8a6 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-reads-from-ets.md @@ -0,0 +1,71 @@ +# Serve Reads from ETS + +When reads vastly outnumber writes, the server should own the data but not serve every read. Readers go straight to an ETS table; only writes go through the mailbox. + +## Avoid + +```elixir +def fetch_flag(server, %FetchFlagRequest{} = request) do + GenServer.call(server, request) +end + +@impl GenServer +def handle_call(%FetchFlagRequest{name: name}, _from, %Flags{} = flags) do + case Flags.fetch(flags, name) do + {:ok, flag} -> {:reply, {:ok, %FetchFlagResponse{flag: flag}}, flags} + {:error, error} -> {:reply, {:error, error}, flags} + end +end +``` + +Every feature-flag check on every request queues behind every other check. + +## Prefer + +```elixir +defmodule MyApp.FeatureFlags do + use GenServer + + alias MyApp.FeatureFlags.{EnableRequest, FeatureFlagsServerState, Flag, FlagNotFoundError} + + def start_link(opts) do + {name, opts} = Keyword.pop!(opts, :name) + GenServer.start_link(__MODULE__, opts, name: name) + end + + def fetch_flag(table, name) do + case :ets.lookup(table, name) do + [{^name, %Flag{} = flag}] -> {:ok, flag} + [] -> {:error, %FlagNotFoundError{name: name}} + end + end + + def enable(server, %EnableRequest{} = request) do + GenServer.call(server, request) + end + + @impl GenServer + def init(opts) do + table = :ets.new(Keyword.fetch!(opts, :table), [:named_table, :protected, read_concurrency: true]) + {:ok, %FeatureFlagsServerState{table: table}} + end + + @impl GenServer + def handle_call(%EnableRequest{name: name}, _from, %FeatureFlagsServerState{} = state) do + :ets.insert(state.table, {name, %Flag{name: name, enabled?: true}}) + {:reply, :ok, state} + end +end +``` + +- `:protected` lets any process read and only the owner write, so writes stay serialized and consistent. +- `read_concurrency: true` optimizes for concurrent readers. +- Reads never reach the GenServer, so `fetch_flag/2` takes domain values and returns the `Flag` struct; Request and Response structs exist only for messages to the server. +- The table is process-only data, so the state is a `FeatureFlagsServerState` wrapper; see [naming.md](naming.md#process-state). +- The caller passes the table name, so several instances can coexist. + +## Trade-offs + +- The table dies with its owner. Readers get `ArgumentError` during the restart window; rebuild the table in `init/1` or `handle_continue/2`, or have a parent process own it. See [resource-lifecycle-ownership.md](resource-lifecycle-ownership.md). +- A write is visible to readers as soon as `:ets.insert/2` returns, and the `call` returns after that, so the writer reads its own write. +- For data that almost never changes, `:persistent_term` reads are faster still, but every update triggers a global garbage collection scan. Use it for configuration, not for anything updated at runtime. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-remove-the-process.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-remove-the-process.md new file mode 100644 index 0000000..184affa --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-remove-the-process.md @@ -0,0 +1,77 @@ +# Remove the Process + +The cheapest queue to fix is the one that should not exist. A GenServer that only forwards to the database, an HTTP client, or a pure function owns nothing, and serializes work that was already concurrent. + +## Avoid + +```elixir +defmodule MyApp.Accounts do + use GenServer + + alias MyApp.Accounts.{GetUserRequest, GetUserResponse, UserNotFoundError} + + def get_user(server, %GetUserRequest{} = request) do + GenServer.call(server, request) + end + + @impl GenServer + def init(_opts), do: {:ok, nil} + + @impl GenServer + def handle_call(%GetUserRequest{id: id}, _from, state) do + case Repo.get(User, id) do + nil -> {:reply, {:error, %UserNotFoundError{id: id}}, state} + user -> {:reply, {:ok, %GetUserResponse{user: user}}, state} + end + end +end +``` + +The state is `nil` forever. Every request in the system waits in this one mailbox while the Repo connection pool sits mostly idle. + +## Prefer + +```elixir +defmodule MyApp.Accounts do + alias MyApp.Accounts.UserNotFoundError + + def get_user(id) do + case Repo.get(User, id) do + nil -> {:error, %UserNotFoundError{id: id}} + user -> {:ok, user} + end + end +end +``` + +The caller's process runs the query. Concurrency is bounded by the Repo pool, which is the resource that actually needs bounding. + +The same holds without Ecto. A Postgrex connection started under the supervisor is already a pool, and any process can query it: + +```elixir +children = [ + {Postgrex, name: MyApp.DB, hostname: "localhost", database: "my_app", pool_size: 10} +] +``` + +```elixir +defmodule MyApp.Accounts do + alias MyApp.Accounts.{User, UserNotFoundError} + + def get_user(id) do + case Postgrex.query(MyApp.DB, "SELECT id, email FROM users WHERE id = $1", [id]) do + {:ok, %Postgrex.Result{rows: [[id, email]]}} -> {:ok, %User{id: id, email: email}} + {:ok, %Postgrex.Result{rows: []}} -> {:error, %UserNotFoundError{id: id}} + {:error, error} -> {:error, error} + end + end +end +``` + +`Postgrex.query/4` checks a connection out of the pool for the caller and returns it after the query, so concurrent callers run concurrent queries up to `pool_size`. HTTP clients work the same way: Finch, and Req on top of it, pool connections and run requests in the caller's process; see [queuing-pool-resources.md](queuing-pool-resources.md). + +## Trade-offs + +None, as long as the process owned nothing. If it was built to make writes fault tolerant or race free, see [anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md) for what provides those guarantees instead. + +If removing it breaks something, that something is what the process owns; see [process-justification.md](process-justification.md). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-shed-load.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-shed-load.md new file mode 100644 index 0000000..18e1f86 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/queuing-shed-load.md @@ -0,0 +1,101 @@ +# Shed Load + +When the queue cannot be removed, bound it. Reject new work early with an error the caller can act on, instead of letting it wait and time out after the server already committed to doing the work. + +## Avoid + +```elixir +def take(server, %TakeRequest{} = request) do + GenServer.call(server, request, 30_000) +end +``` + +A longer timeout hides the overload: callers wait longer, the mailbox keeps growing, and work for callers that already gave up still runs. + +## Prefer + +```elixir +@max_queue_length 1_000 + +def take(server, %TakeRequest{} = request) do + if overloaded?(server) do + {:error, %OverloadedError{}} + else + GenServer.call(server, request) + end +end + +defp overloaded?(server) do + with pid when is_pid(pid) <- GenServer.whereis(server), + {:message_queue_len, length} <- Process.info(pid, :message_queue_len) do + length > @max_queue_length + else + _ -> false + end +end +``` + +- The check runs in the caller, so it costs the server nothing. +- `{:error, %OverloadedError{}}` follows the reply shapes in [callback-patterns.md](callback-patterns.md#reply-shapes); callers map it to a retry with backoff or an HTTP 429. +- When the server is not running, `overloaded?/1` returns `false` and the `call` exits as it would have anyway. + +The mailbox check is best-effort: concurrent callers can all see a length below the limit and enqueue together, so it cannot enforce a hard bound. + +## Hard Bound + +Count requests from the moment a caller sends one until the server takes it out of the mailbox. The caller reserves a slot, and the server releases it: + +```elixir +@max_queued 1_000 +@timeout 5_000 + +def take(name, %TakeRequest{} = request) do + case Registry.lookup(MyApp.RateLimiters, name) do + [{pid, admission}] -> admit(pid, admission, request) + [] -> {:error, %RateLimiterNotFoundError{name: name}} + end +end + +defp admit(pid, admission, %TakeRequest{} = request) do + if :atomics.add_get(admission, 1, 1) > @max_queued do + :atomics.sub(admission, 1, 1) + {:error, %OverloadedError{}} + else + deadline = System.monotonic_time(:millisecond) + @timeout + GenServer.call(pid, %TakeRequest{request | admission: admission, deadline: deadline}, @timeout) + end +end + +@impl GenServer +def init(opts) do + admission = :atomics.new(1, signed: true) + {:ok, _owner} = Registry.register(MyApp.RateLimiters, Keyword.fetch!(opts, :name), admission) + {:ok, Bucket.new(Keyword.fetch!(opts, :capacity))} +end + +@impl GenServer +def handle_call(%TakeRequest{} = request, _from, %Bucket{} = bucket) do + :atomics.sub(request.admission, 1, 1) + + if System.monotonic_time(:millisecond) > request.deadline do + {:noreply, bucket} + else + case Bucket.take(bucket, request.key) do + {:ok, bucket} -> {:reply, {:ok, %TakeResponse{remaining: Bucket.remaining(bucket, request.key)}}, bucket} + {:error, error} -> {:reply, {:error, error}, bucket} + end + end +end +``` + +- The slot is released when the request leaves the mailbox, not when the caller stops waiting. A caller that times out leaves its request queued and its slot taken, so the count never drops below what the mailbox holds. +- The server creates the counter in `init/1` and publishes it as its `Registry` value. A restart drops the old entry with the old process and starts from a fresh counter, so slots held by requests lost in the crash do not leak into the new server. See [resource-lifecycle-ownership.md](resource-lifecycle-ownership.md). +- Callers send to the pid they looked up, and the request carries the counter it reserved, so the server always releases the right one. +- The deadline lets the server drop a request whose caller already gave up, without doing the work; `:noreply` avoids a late reply to it. + +## Trade-offs + +- Use the mailbox check when stopping runaway growth is enough; use admission control when the bound must hold. +- `Process.info/2`, `:atomics`, `Registry`, and `System.monotonic_time/1` are local to one node, so both checks fit callers on the server's node. +- Admission control counts only callers that go through it; casts and messages from elsewhere still reach the mailbox. +- Shedding is a last resort. It keeps the node alive, but callers still fail; prefer the patterns that remove the queue. diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/request-response.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/request-response.md new file mode 100644 index 0000000..000466b --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/request-response.md @@ -0,0 +1,83 @@ +# Request and Response Contracts + +The client API is a contract with every caller. Shape it so it can grow without breaking them: wrap what goes in and what comes out in structs, never bare values. + +## Avoid + +```elixir +def recent_messages(room_id) do + GenServer.call(via(room_id), :recent_messages) +end + +@impl GenServer +def handle_call(:recent_messages, _from, %Room{} = room) do + {:reply, {:ok, Room.recent_messages(room)}, room} +end +``` + +```elixir +{:ok, messages} = RoomServer.recent_messages(room_id) +``` + +The day the room needs to page back through its history, there is nowhere to put the page size or the marker for "more". Every option breaks callers: + +- `{:ok, messages, more?}` breaks every match and violates the [reply shapes](callback-patterns.md#reply-shapes). +- `{:ok, %{messages: messages, more?: more?}}` breaks every caller that matched a list. +- `recent_messages(room_id, limit)` changes the arity, and every call site. + +## Prefer + +```elixir +defmodule MyApp.Chat.ListMessagesRequest do + defstruct limit: 50, before: nil +end + +defmodule MyApp.Chat.ListMessagesResponse do + defstruct messages: [], more?: false +end +``` + +```elixir +def list_messages(room_id, %ListMessagesRequest{} = request) do + GenServer.call(via(room_id), request) +end + +@impl GenServer +def handle_call(%ListMessagesRequest{} = request, _from, %Room{} = room) do + %History{} = history = Room.history(room, request.before, request.limit) + {:reply, {:ok, %ListMessagesResponse{messages: history.messages, more?: history.more?}}, room} +end +``` + +```elixir +{:ok, %ListMessagesResponse{messages: messages}} = + RoomServer.list_messages(room_id, %ListMessagesRequest{}) +``` + +New request options and new response fields are new struct fields with defaults. Existing callers keep compiling and keep matching. + +The core follows the same idea one layer down: `Room.history/3` returns a `Room.History` struct, never a loose `{messages, more?}` tuple. The room is a process per entity that keeps a bounded history; see [queuing-process-per-entity.md](queuing-process-per-entity.md). + +## Growing Without Breaking + +A change to the contract should be additive: a new field with a default. A shape that can only change by being replaced forces every caller to change at once. That one idea explains every choice below. + +- **A Request, even with no fields.** An operation that takes nothing today takes a filter or a page size tomorrow. A field is additive; a new argument changes the arity. +- **A Response, not a bare value.** A bare list, map, integer, or boolean has no room for a second piece of data, so the first piece of metadata (cursor, total, truncation) replaces the shape. +- **Collections in a named field.** Data about a collection belongs next to it, and a field named `messages` leaves room for `more?`. +- **`:ok` only when there is never anything to say.** Moving from `:ok` to `{:ok, response}` is a replacement. When an operation might return data later, start with an empty Response struct. +- **Callers match what they read.** A struct pattern such as `%ListMessagesResponse{messages: messages}` ignores fields added later. Comparing whole structs with `==`, in code or in test assertions, breaks as soon as a new field carries a value. +- **Evolve by addition.** Renaming or removing a field is a replacement. Add the new field and keep the old one until callers move. +- **Enforced keys are decided once.** Adding `@enforce_keys` later breaks every caller that builds the struct without the key. +- **Errors grow the same way.** `defexception` structs carry new detail as new fields; see [naming.md](naming.md#errors). + +## Boundaries + +Request and Response structs exist only at the GenServer boundary. `handle_call/3` unpacks the request and calls the pure core with domain values; the core returns domain values that the server wraps in the response. See [naming.md](naming.md#messages). + +Items inside a response are domain structs (`%Message{}`), not tuples, for the same reason the reply is a struct. + +## Trade-offs + +- A struct per operation is more modules. That cost is paid once; a breaking change is paid at every call site. +- A response struct does not make a large reply cheap; it is still copied to the caller. Return a page or a lookup, not the whole collection. See [queuing-large-replies.md](queuing-large-replies.md). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/resource-lifecycle-ownership.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/resource-lifecycle-ownership.md new file mode 100644 index 0000000..4567b49 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/resource-lifecycle-ownership.md @@ -0,0 +1,134 @@ +# Resource Lifecycle Ownership + +When a resource should live exactly as long as a process, tie it to that process and let the exit release it. Exit is the one cleanup path that always runs: a `:kill`, a VM crash, or an exit signal received without trapping exits (including a supervisor shutdown) skips `terminate/2`, and a crashed caller never reaches its `release/1`, but none of them skips the exit. + +## What the Runtime Cleans Up + +| Resource | When the owning process exits | +|----------|-------------------------------| +| ETS table | Deleted, or given to its `heir` | +| Port or socket | Closed | +| `Registry` key | Unregistered | +| Monitor | The watcher receives `:DOWN` | +| Link | Linked processes receive an exit signal | + +Files, rows, external leases, and anything else outside the VM are not on this list. The runtime cannot release them; see [External Resources](#external-resources). + +## Two Roles + +- **Owner:** the process is the lifetime. It holds the resource, and the resource ends when the process ends. A connection process owns its socket; a room owns its ETS table. +- **Observer:** the process watches someone else's lifetime. It monitors that process and releases the resource on `:DOWN`. + +Neither role serves calls on the hot path, so neither becomes a queue. Both satisfy "A Process Must Own Something": the process owns a lifecycle. + +## Avoid + +```elixir +@impl GenServer +def handle_call(%AcquireRequest{key: key}, {pid, _tag}, %LockServerState{} = state) do + if Map.has_key?(state.holders, key) do + {:reply, {:error, %LockHeldError{key: key}}, state} + else + {:reply, :ok, %LockServerState{state | holders: Map.put(state.holders, key, pid)}} + end +end + +@impl GenServer +def handle_call(%ReleaseRequest{key: key}, _from, %LockServerState{} = state) do + {:reply, :ok, %LockServerState{state | holders: Map.delete(state.holders, key)}} +end +``` + +The lock is released only when the holder remembers to call `release`. A holder that crashes, times out, or is killed holds the lock forever, and every caller queues behind one process to acquire it. + +## Prefer + +Let the holder own the lock. A unique `Registry` key belongs to the process that registered it and disappears when that process exits: + +```elixir +children = [{Registry, keys: :unique, name: MyApp.Locks}] +``` + +```elixir +defmodule MyApp.Locks do + def acquire(key) do + case Registry.register(__MODULE__, key, nil) do + {:ok, _owner} -> :ok + {:error, {:already_registered, holder}} -> {:error, %LockHeldError{key: key, holder: holder}} + end + end + + def release(key), do: Registry.unregister(__MODULE__, key) +end +``` + +- `release/1` is an optimization for holders that finish early. Correctness never depends on it. +- There is no lock server to queue behind. `Registry` partitions its own bookkeeping. +- `Registry` is local to one node. A lock that must hold across nodes belongs in the database; see [anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md). + +## External Resources + +When the resource lives outside the VM, a process still gives it a lifetime, but the release is code that can be skipped. Make the process an observer of the real owner and keep a backstop for the cases it misses: + +```elixir +defmodule MyApp.Uploads.ScratchDirServer do + use GenServer, restart: :temporary + + def start_link(%ScratchDir{} = scratch_dir) do + GenServer.start_link(__MODULE__, scratch_dir) + end + + @impl GenServer + def init(%ScratchDir{} = scratch_dir) do + Process.flag(:trap_exit, true) + owner_ref = Process.monitor(scratch_dir.owner) + + case File.mkdir_p(scratch_dir.path) do + :ok -> {:ok, %ScratchDirServerState{scratch_dir: scratch_dir, owner_ref: owner_ref}} + {:error, reason} -> {:stop, reason} + end + end + + @impl GenServer + def handle_info({:DOWN, ref, :process, _pid, _reason}, %ScratchDirServerState{owner_ref: ref} = state) do + {:stop, :normal, state} + end + + @impl GenServer + def terminate(_reason, %ScratchDirServerState{} = state) do + File.rm_rf(state.scratch_dir.path) + end +end +``` + +```elixir +def open_scratch_dir(owner \\ self()) do + path = Path.join(System.tmp_dir!(), "upload-#{System.unique_integer([:positive])}") + scratch_dir = %ScratchDir{owner: owner, path: path} + + case DynamicSupervisor.start_child(MyApp.ScratchDirs, {ScratchDirServer, scratch_dir}) do + {:ok, _pid} -> {:ok, scratch_dir} + {:error, error} -> {:error, error} + end +end +``` + +- The directory follows the owner, a request or a LiveView, without the owner calling anything. When the owner exits, the server stops and removes it. +- `restart: :temporary`: a restarted server would watch nothing, and its owner may already be gone. +- Trapping exits makes `terminate/2` run on supervisor shutdown, within the child spec `shutdown`. +- A monitor, not a link: the owner's crash should stop the server cleanly, and the server's crash should not take the owner down. +- `terminate/2` is still skipped by `:kill` and by a VM crash. Sweep stale directories when the application boots; the sweep is what makes the cleanup certain, the server is what makes it prompt. + +## Smells in Review + +- Cleanup depends on callers remembering to call `release`, `close`, or `stop` +- `terminate/2` is the only place a resource is released +- State holds pids with no monitor, so entries for dead processes stay forever +- A link where a monitor belongs: the observer dies with the process it watches +- A process recreates, on restart, a resource whose owner is gone + +## Trade-offs + +- An ETS table dies with its owner; give it an `heir` or a longer-lived parent when readers must survive a crash. See [queuing-reads-from-ets.md](queuing-reads-from-ets.md). +- One observer per resource is a process per resource. That is cheap, but it is still a process to supervise; a single observer that monitors many owners fits when the resources are small and uniform. See [handle-info.md](handle-info.md#monitors). +- When `terminate/2` runs and when it does not is covered in [supervision-and-testing.md](supervision-and-testing.md#cleanup). diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/review-checklist.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/review-checklist.md new file mode 100644 index 0000000..e01face --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/review-checklist.md @@ -0,0 +1,65 @@ +# Review Checklist + +Each item is a symptom of a principle, not a rule on its own. When an item fails, read the linked reference and judge the code by the principle behind it; a justified exception passes, a compliant design that misses the point does not. + +## Justification ([process-justification.md](process-justification.md)) + +- [ ] The process owns something concrete: shared state, a resource, a lifecycle, or a failure boundary +- [ ] No stateless operation (DB, HTTP, computation) is routed through it + +## Resource Lifecycle ([resource-lifecycle-ownership.md](resource-lifecycle-ownership.md)) + +- [ ] Resources are released by process exit, not only by an explicit call or `terminate/2` +- [ ] Resources outside the VM have a boot-time sweep as a backstop + +## Database Writes ([anti-pattern-database-gatekeeper.md](anti-pattern-database-gatekeeper.md)) + +- [ ] Write correctness comes from constraints, transactions, and locks, not from routing writes through one process +- [ ] No writes that must survive a crash go through `cast` or a mailbox +- [ ] State does not cache rows that other code also writes +- [ ] Write-behind buffers document their loss window and flush on shutdown + +## Queuing ([mailbox-queuing.md](mailbox-queuing.md)) + +- [ ] Hot-path throughput fits within `1 / handling_time` of one process, or the work is partitioned +- [ ] No unbounded casts from producers that can outrun the server +- [ ] No blocking I/O inside callbacks on a hot path +- [ ] Replies return only what the caller needs, not whole collections +- [ ] Handling time and mailbox length are measured with telemetry + +## Module Shape ([callback-patterns.md](callback-patterns.md)) + +- [ ] Client API wraps every `call`/`cast`; callers never send messages directly +- [ ] Callbacks delegate to pure, separately tested functions +- [ ] `init/1` is fast; slow work runs in `handle_continue/2`, unless siblings need the state at boot +- [ ] Persistent dependency failures retry with backoff instead of crash-looping the supervisor +- [ ] Replies and fallible core results are `:ok`, `{:ok, value}`, or `{:error, error}`, never longer tuples; core functions that cannot fail return a value or a struct +- [ ] Process names never built from untrusted input +- [ ] Secrets in state are redacted with `format_status/1` (Elixir 1.17+; `format_status/2` before that) + +## handle_info ([handle-info.md](handle-info.md)) + +- [ ] Callbacks never call the module's own client API +- [ ] Self-sent and timer messages are structs; OTP and library tuples are matched as defined +- [ ] Timers use `Process.send_after/3` with a token to ignore stale ticks +- [ ] Pids held in state are monitored and removed on `:DOWN` +- [ ] `handle_info/2` ends with a catch-all clause + +## Naming ([naming.md](naming.md)) + +- [ ] Pure core named after a domain concept, not a role (`State`, `Core`, `Logic`, `Impl`) +- [ ] The domain struct is the state; a `ServerState` wrapper exists only for process-only data or several domain structs +- [ ] Server modules carry the domain name (`RateLimiterServer`), never a bare `.Server` +- [ ] GenServer messages are `Request` / `Response` structs, not tuples +- [ ] The pure core never references the Request/Response structs + +## Contracts ([request-response.md](request-response.md)) + +- [ ] Every operation takes a Request struct, even with no fields +- [ ] Replies carry a Response struct, never a bare list, map, or scalar +- [ ] Collections sit in a named field of the Response +- [ ] New fields have defaults; no field callers read was renamed or removed + +## Supervision ([supervision-and-testing.md](supervision-and-testing.md)) + +- [ ] Restart strategy matches the process lifecycle diff --git a/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/supervision-and-testing.md b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/supervision-and-testing.md new file mode 100644 index 0000000..e635601 --- /dev/null +++ b/plugins/trogonstack-elixir/skills/elixir-design-genserver/references/supervision-and-testing.md @@ -0,0 +1,35 @@ +# Supervision and Testing + +A restart repairs state and nothing else: it fixes corruption and transient faults, and repeats any fault that lives outside the process. Supervise for the faults a clean start fixes, and test logic where it lives, in the pure core. + +## Restart Strategy + +`use GenServer` generates `child_spec/1` with `restart: :permanent`. Override it when that is wrong: + +```elixir +use GenServer, restart: :transient +``` + +| Strategy | Restarts when | Fits | +|----------|---------------|------| +| `:permanent` | Always | Long-lived services | +| `:transient` | Abnormal exit only | Work that can finish normally, per-entity processes that reload durable state | +| `:temporary` | Never | One-shot processes whose failure the caller handles, in-memory per-entity processes | + +## Cleanup + +- `terminate/2` runs on supervisor shutdown only if the process traps exits (`Process.flag(:trap_exit, true)`) and finishes within the child spec `shutdown` value. +- Trap exits only when cleanup is required, and set an explicit `shutdown`. +- Do not rely on `terminate/2` for anything that must happen; a `:kill` skips it. Tie the resource to a process exit instead; see [resource-lifecycle-ownership.md](resource-lifecycle-ownership.md). + +## State Across Restarts + +A restart starts from `init/1` with fresh state. If state must survive a crash, persist it, or rebuild it in `handle_continue/2` from a durable source. + +## Testing + +- Test the pure core directly, without processes. Most logic lives there. +- Start servers with `start_supervised!/1` so ExUnit stops them between tests. +- Pass a unique `:name` per test to keep `async: true`. +- Assert through the client API. Use `:sys.get_state/1` only as a last resort. +- For queuing behavior, measure handling time of the pure core and reason with `1 / handling_time` rather than load-testing the process in unit tests.