Creating or attaching a client across N inbounds called AddInboundClient
once per inbound, strictly one after another. When those inbounds live on
different nodes each call is a full node round-trip bounded by the 10s
remote timeout, so the request cost the SUM of every node's latency: two
nodes felt instant, three took ~13s and timed out bot callers, which is
how it surfaced as "two out of four account creations fail".
Split the per-inbound preparation from the apply. Preparation stays
ordered and single-threaded because fillProtocolDefaults mints the shared
credentials on the first inbound and every later one reuses them; the
applies then run concurrently, capped at inboundFanoutConcurrency. A
4-node create measured 1.205s -> 0.307s with peak overlap 1 -> 4.
Consequences of no longer aborting at the first failing inbound:
- Every apply error is tagged with its inbound and the failures are
joined, so all of them reach the caller instead of just the first.
- The fanout goroutines recover their own panics. Off the request
goroutine gin's Recovery no longer covers them, and an unrecovered
panic would kill the panel rather than fail one inbound.
- A partly-applied call commits clients on the inbounds that succeeded,
so the controller and the LDAP job now read needRestart before the
error check; otherwise Xray was never flagged for the work that landed.
- limitHwid is applied only when every inbound succeeded. Applying it
after a failure rewrites limit_hwid and trims the registered devices of
an email that already existed, which is silent data loss on an
operation the panel reported as failed.
Update the API docs for the new partial-application contract and the
inbound-tagged error strings.
Create and Attach called the exported AddInboundClient once per target
inbound, and that wrapper passes a nil email→subId map, so every iteration
re-ran getAllEmailSubIDs -- a JSON_EACH expansion over the settings blob of
every inbound in the panel. Adding one client to 24 inbounds on a panel with
~300 users meant 24 full expansions of ~7k rows to answer the same question.
Hoist the snapshot above the loop and call the unexported addInboundClient
with it, exactly as BulkAttach (client_bulk.go:63) and BulkCreate
(client_bulk.go:1151) already do. The snapshot goes stale from the second
inbound onward, but the identity being added is the same on every iteration,
so its own entry can only ever match itself -- checkEmailsExistForClients
accepts an email whose stored subId equals the incoming one, and an absent
entry is accepted too.
This is the database half of #6091. The dominant cost there is the other
half -- one synchronous 10s-capped node round-trip per remote inbound,
which multiplies again on chained nodes -- and that needs the push batched
per node rather than per inbound; left for a separate change.
Sanaei