Commit Graph

3 Commits

Author SHA1 Message Date
Sanaei a5e68f410f perf(node): bound the per-client node push and fan out the traffic reset
An operator with several nodes reported that editing a client or resetting
its traffic takes more than ten seconds on the master. Measured against real
Remote HTTP (fake node servers, one client per node), the healthy case is
already fast — 3 nodes: update 51ms, delete 51ms; 5 nodes: 102ms / 103ms —
but two things were not:

  - ResetTrafficByEmail still walked its inbounds one node round-trip after
    another: 152ms at 3 nodes, 253ms at 5, linear in node count.
  - Every per-client op blocked on the SLOWEST node's push. With one node
    answering in 3s, update/delete/reset all took 3003ms regardless of node
    count. A node that answers the 4s heartbeat probe but hangs on the push
    stays "online", so every edit waited on it up to remoteHTTPTimeout — the
    ten seconds in the report. More nodes only raise the odds one is sick.

The push is an immediacy optimisation, not the source of truth: every one of
these ops calls MarkNodeDirtyTx inside the transaction that commits the
change, before it pushes, and the node reconcile job converges a dirty node on
its next 5s tick by re-sending the inbound whose fingerprint was not advanced.
So bound the synchronous push with nodeClientPushTimeout = 4s — the budget the
heartbeat and traffic-sync jobs already treat as "responsive" — at the eight
node-branch push sites. A node that does not answer in time is left dirty and
converged a few seconds later instead of stalling the request; the tag-cache
list fetch inside resolveRemoteID shares the same budget.

Once one push in a batch has timed out, the rest of that inbound's batch now
stops pushing too, as AddInboundClient already did: the node is dirty and one
reconcile converges the whole inbound. Deleting three clients on one hung node
went from 30.08s (three remote timeouts) to 4.06s; at the 32-client push
threshold that is 128s of deadlines saved per inbound.

Fan the reset out through fanoutInboundApplies like the other client ops. Its
node propagation is still attempted whatever the node's status flag says, as
before, because nothing replays a traffic reset — the reconcile pushes inbound
config, not counters — so a node still serving after being marked offline must
receive it now or never.

Trade-offs stated plainly: a node that would have answered in 4–10s now falls
to the reconcile's full-inbound push, which on the node is a delete+add of the
inbound and drops its sessions there — the same fallback a failed 10s push
already used, now reached sooner. The reset stays best-effort with no retry
path, which predates this change. The response still reports success while a
timed-out node catches up; the pending-node badge is keyed off node status by
design, so only the warning log records it.

Tests: a barrier test that a sequential reset cannot satisfy; two tests against
a real runtime.Remote and an httptest node that hangs on the push, pinning that
an edit returns at the deadline (exactly one push reached the node, the node
is left dirty) and that a bulk delete stops after its first timed-out push.
All red without the change; the two hung-node tests pay their 4s deadline on
every run.
2026-09-07 14:24:14 +02:00
Sanaei e9e2e30278 perf(clients): push a bulk client change to every node at once
63b46cd6 made a multi-inbound client create apply its inbounds concurrently,
and d34ec97f did the same for the single-client update, delete and detach. The
bulk operations were never converted, so they still walked their inbounds in a
plain sequential loop with a node RPC in each iteration — and those are what the
panel actually calls for a multi-select delete or an enable/disable, which is
why editing and deleting still felt slow on a master with several nodes.

Measured with a node runtime injecting 100ms per RPC, one client per node:

  nodes=1  update=101ms  bulkSetEnable=101ms  bulkAdjust=101ms  bulkDelete=101ms
  nodes=3  update=102ms  bulkSetEnable=302ms  bulkAdjust=304ms  bulkDelete=303ms
  nodes=5  update=203ms  bulkSetEnable=504ms  bulkAdjust=504ms  bulkDelete=504ms

after, all of them track the single-client ops:

  nodes=3  bulkSetEnable=103ms  bulkAdjust=101ms  bulkDelete=101ms
  nodes=5  bulkSetEnable=202ms  bulkAdjust=202ms  bulkDelete=202ms

Generalize the fanout into fanoutInboundResults over an arbitrary per-inbound
result type and route six loops through it: BulkDelete, BulkSetEnable,
BulkAdjust, BulkDetach, BulkAttach, BulkCreate, plus applyClientFieldByEmail —
the field edit behind the Telegram bot's enable/limit/expiry buttons and the
LDAP job. Each keeps its preparation sequential and overlaps only the node
pushes, inheriting the same concurrency cap and per-inbound panic recovery.

Two ordering details the sequential loops got for free and the fanout must do
itself: the three loops that ranged a map now walk sortedInboundIds, so which
inbound wins a per-email skip reason is the lowest id instead of whatever the
map yielded; and BulkAttach de-duplicates a repeated inbound id up front,
because the second pass used to see the client the first pass had just added.

The allocating paths stay serial when a tunnel inbound is involved. WireGuard
and AmneziaWG pick a free peer address by reading every inbound's used-set
before they write, so two overlapping allocations hand out the same address and
the in-transaction re-check refuses the loser — a bulk create of two clients
onto two wg inbounds returned created=1. addFanoutLimit drops those batches back
to one at a time; every other protocol keeps the full cap.

Eight tests: seven barrier tests that a sequential caller cannot satisfy (peak
pushes in flight is 1 without the change, 4 with it), and one that pins the
tunnel allocation.
2026-09-07 02:19:16 +02:00
Sanaei d34ec97f62 perf(node): push a client edit to every node at once, not one after another
Editing, deleting or detaching a client on a master with several nodes took
one node round-trip per node, added end to end. Create and Attach already
fanned their per-inbound applies out through fanoutInboundClientAdds, but
Update, Delete, Detach and DeleteByEmail's record-less fallback still walked
their inbounds in a plain sequential loop, and each iteration blocks on a
node RPC (10s timeout, more when a node is slow or has just gone unreachable
and the heartbeat has not marked it offline yet).

Measured with a node runtime injecting 100ms per RPC, before:

  nodes=1  create=101ms  update=101ms  delete=101ms
  nodes=3  create=102ms  update=303ms  delete=302ms
  nodes=5  create=202ms  update=504ms  delete=504ms

after, all three track create:

  nodes=3  create=102ms  update=102ms  delete=101ms
  nodes=5  create=203ms  update=203ms  delete=203ms

Generalize the existing helper into fanoutInboundApplies over an inboundApply
list and route the four remaining loops through it, so they inherit the same
concurrency cap, per-inbound panic recovery and joined errors. Each caller
still builds its payloads sequentially first: fillProtocolDefaults mints the
shared credentials on the first inbound and every later one reuses them, so
that order has to stay deterministic. Only the applies overlap; their DB work
still serializes through the single traffic writer, and the per-inbound
mutation lock is unchanged, which is exactly what Create has relied on.

Behaviour change: one failing inbound no longer aborts the remaining ones,
matching what Create already does. The error still names each failed inbound
and the record-level writes are still skipped when any inbound failed.

The snapshot merge on the same serialized writer was measured as a second
suspect and cleared: ~43ms per node at 500 clients, an order of magnitude
below the RPC serialization.
2026-09-04 11:39:56 +02:00