Repository navigation
fix(controller): let a core start outlive the Start request - #116
Merged
Merged
Conversation
Xray startup waited for the core's API under the request context. The panel gives up on Start after its RPC timeout (10 s by default), while a 163k-user start takes about 13 s even on a fast host, so the node killed the core it was starting and every retry failed the same way. The panel already expects a timed-out Start to finish on the node and tries to attach after a timeout, but found nothing to attach to. StartOrAttach now runs under a context that keeps the request's values but not its cancellation, bounded by a 3 minute start timeout. A Start whose caller left ends with the core running and the keep-alive session started, and a Start that arrives meanwhile waits for it and attaches. Disconnect, which process shutdown calls without LockControl, aborts a start in progress so it does not wait for it. A full SyncUsers had the same problem: when the caller gave up while the restarted core was booting, the node restarted xray a second time on the old config and dropped the new users. It now waits for the new core regardless; Shutdown still ends the wait.
…t; gate attach test Note the controlMu/mu/startMu order, that abortStart is a no-op under LockControl, that fullSyncReadyTimeout covers both callers, and make the attach test wait for a start in progress before the retry.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…uest # Conflicts: # controller/controller.go
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
d309483 (PasarGuard#115) is now ported by c2e15f2 and its waiver goes. 47857d4 (the v0.5.5 version bump) is not applicable: the fork keeps its own NodeVersion line. bb40094 (PasarGuard#111) is superseded while the fork pins Xray v26.3.27 and checks the geo files by sha256. d359a8f (PasarGuard#117) is not applicable without StartOrAttach. bd745b5 (PasarGuard#118) is not applicable until PasarGuard#64 brings Xray WireGuard accounts and goes with it. 1549c96 (PasarGuard#116) and 8ad52fe (PasarGuard#112) stay pending to 2026-11-08 under NODE-LEAD, each with its exact remaining work.
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…asarGuard#112 waivers The pending lines for 1549c96 (PasarGuard#116) and 8ad52fe (PasarGuard#112) cited six candidate commits that exist only on a local branch and resolve nowhere on GitHub (finding F-V7-V5). The remaining work they describe is unchanged.
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…sked for it (upstream PasarGuard#116) Upstream PasarGuard/node 1549c96 (PasarGuard#116, released in v0.5.5): a core start waited for xray's API under the request's context. The panel gives up on Start after its RPC timeout (10 s on most nodes), so a start that takes longer (a large user list, a node under memory pressure, the filter fallback's retries) killed the core it was building, after the old core was already gone, and every retry failed the same way. A full SyncUsers whose caller gave up while the new core booted rolled back and restarted xray a second time on the old users. Failing on the parent: - TestAStartOutlivesTheCallerThatGaveUp: a start that takes 400 ms with a caller that leaves after 50 ms fails with "context canceled". - TestSyncUsersOutlivesTheCaller (xray, upstream's test on the fork's core fields): "SyncUsers failed after its caller went away: context cancelled", after a second restart. - TestDisconnectAbortsAStartInProgress, TestCloseAbortsAStartInProgress and TestDisconnectAbortsAnXrayStartInProgress (upstream's, through ReplaceBackend and a fake xray that never answers): Disconnect and Close wait behind the start. On the parent a start ends when its request does, and these callers never cancel; once a start no longer follows its caller, Disconnect and Close (which process shutdown calls without the control lock) must abort it, and these pin that. Upstream's tests are carried without their comments (no explanatory comments in this fork). Its TestStartOutlivesItsCaller needs a real xray binary; the fork version drives the same case through the controller's backend builder seam. TestSecondStartAttachesToAStartInProgress is not carried: this fork's Start never attaches, it always rebuilds the core from the Start's user list. Upstream: 1549c96
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…tream PasarGuard#116) Port of upstream PasarGuard/node 1549c96 (PasarGuard#116, released in v0.5.5), reconciled with this fork's user-sync lifecycle fence. Start: ReplaceBackend, which both the gRPC and the REST Start call, now runs the start under a context that keeps the request's values but not its cancellation, bounded by a 3 minute start timeout (an expired one is reported as "core not ready within 3m0s"). A Start whose panel gave up ends with the core running and Connect called, and the fork panel's attach-after-timeout path finds it. Upstream puts this in StartOrAttach, which this fork does not carry; the Start still rebuilds the core from the Start's user list every time. Abort: Disconnect and Close cancel a start in progress before they wait for the lifecycle lock. Process shutdown calls Close without the control lock (cmd/node/main.go); Close also marks the controller closing, so a start that registers after Close began is cancelled at once. Under the control lock (Stop, the keep-alive watchdog) there is never a start to abort, as upstream notes. Full sync: applyConfigWithRestart waits for the new core's API under backend.DetachFromCaller(ctx, 3m) instead of the caller's context, so a panel that gives up while the core boots no longer causes a rollback and a second restart on the old users. Shutdown still ends the wait, since checkXrayStatus sees the core stop. Reconciled with the lifecycle fence (af180e2): a user sync holds the lifecycle lock as a reader, and a Start or Close that waits longer than the drain grace cancels the in-flight syncs (cancelAll, pinned by TestAUserSyncWedgedInsideACoreDoesNotHoldStartHostage). Upstream's context.WithoutCancel would hide that cancel too, and an abandoned full sync would then hold a Start for up to 3 minutes. Each tracked sync now carries its own abort context (backend.WithSyncAbort), which cancelAll cancels, and DetachFromCaller ignores only the caller leaving (a plain Canceled or DeadlineExceeded) while it follows that abort and any other cancellation cause, such as a health restart's stuck-sync abort on release/v0.6.21-acct-xrayrace, whose merge must pass its cause through the same way. Tests added here: the DetachFromCaller unit tests, TestAFullSyncWhoseCallerLeftDoesNotHoldAStartHostage (controller) and TestAFullSyncWhoseCallerLeftStillYieldsToALifecycleAbort (xray, fake core). Both fail when DetachFromCaller ignores the sync abort, the plain WithoutCancel of upstream. Not carried: AttachBackend (the fork's AddBackend) still builds under the request's context; detaching it changes what a panel retry sees and is left to the owner. Upstream's comments are not carried (no explanatory comments in this fork). Upstream: 1549c96
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…asarGuard#116 scope) Upstream PasarGuard#116 detaches only the core start of a Start request; this branch carries that in ReplaceBackend. AddBackend (AttachBackend) is this fork's own call and keeps building the extra backend under the request's context, so a request the panel cancels also cancels the build, and nothing is left attached for the panel's retry to trip over as "already running". Detaching it would change exactly what such a retry sees, so it stays as it is (owner decision 2026-10-26, F-SYNC055-ATTACHCTX: keep upstream's behaviour). TestAnAddedBackendStopsBuildingWhenItsRequestIsCancelled pins it: with a primary running, an AddBackend whose request is cancelled after 50 ms must return an error within a second and leave only the primary. It passes on this branch and fails when AttachBackend runs under detachStart like ReplaceBackend ("AddBackend succeeded after its request was cancelled (took 3.000766792s)").
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…onnect (upstream PasarGuard#116) Sixteen rounds under -race, alternating a Disconnect that queues on the control lock like the RPC Stop with one that calls Disconnect directly at staggered delays. Each round checks that all three finish, that only an abort can fail a start, that no stray core runs beside the attached backend, that an attached backend is the second start's running core, and that a second start which finished last stays attached. Starting the core under the request context fails it in round 0.
Free-Guy-IR
added a commit
to Free-Guy-IR/node
that referenced
this pull request
Nov 3, 2026
…y trailer 1549c96 (PasarGuard#116) is ported by 58a47bd and 8ad52fe (PasarGuard#112) by 74e9aaa and ee73b4f, each carrying an Upstream: trailer; the drift check reports both as PORTED, so their pending waivers are removed.
T3ST3ST3R0N
added a commit
that referenced
this pull request
Nov 4, 2026
* feat(proto): add MTPROTO backend type and Masque and Mtproto proxy messages Add BackendType MTPROTO = 2, the Masque message (matching the released Python bridge 0.9.3) and the Mtproto user message, exposed on Proxy as masque = 7 and mtproto = 8. Regenerate the Go code. * feat(mtproto): add telemt MTProto backend Port node PR #61 (feat/mtproto-telemt, head 4eb26fb) onto dev: the backend/mtproto package that drives telemt as a child process, the MTPROTO case in Controller.StartBackend, TELEMT_EXECUTABLE_PATH (default /usr/local/bin/telemt) in config and .env.example, and the install_telemt Makefile target (TELEMT_VERSION still 3.4.25). Ported PR commits: 8542f27, 90c50a1, 419732c, 9f9eac2, a87168d, 77c62fd, 630df12, 0eae051, ef43229, cd183a4, a66a322, fa79e3b, 098d828, 2c4fce8, 4eb26fb. The proto part of 8542f27 landed in the previous commit. Dockerfile.mtproto (9f9eac2, 4eb26fb) is not ported. Adapt the backend to dev's Backend interface: the process manager now keeps its output in a logstream.Buffer, the capture goroutines Publish into it, MTProto.SubscribeLogs returns a Subscribe channel, and Shutdown closes the buffer. This replaces Logs(), CloseLogs and the log WaitGroup, so a late log line can no longer be sent on a closed channel. Add a test that runs a stand-in process and checks that its stdout and stderr reach a subscriber and that shutdown closes the subscription cleanly. Co-authored-by: x0sina <bulletsina77@gmail.com> * fix(mtproto): harden rendered telemt config and skip invalid users Config: - Reject panel JSON that sets node-managed or host-reaching telemt keys: include, web, server.api, server.admin_api, server.metrics_port, server.metrics_listen, server.metrics_whitelist, access.user_rate_limits and access.user_source_deny (next to the existing restricted per-user access maps). - Validate object sections and ports (1-65535, integers) and add Config.ListenPorts (server.port, default 443, plus listener ports). - Render forces general.disable_colors, general.telemetry.user_enabled and server.api.read_only=false, and fails when a listen port equals the node's telemt API or metrics port. TOML: - Render strings and keys as TOML basic strings (\uXXXX for control characters) instead of strconv.Quote, whose \a, \v and \x escapes are not TOML. - Drop null object fields; a null array element stays an error. - Always quote a key named include: telemt expands any `include = "..."` line as a file include before parsing. Users: - ActiveUsersFromCommon logs and skips invalid users and returns their emails, so one bad user no longer fails Start or a full sync. Partial updates treat an invalid user as not included. SyncUser still returns the validation error for its single user. Tests use github.com/pelletier/go-toml v1.9.5 (already in go.sum) as an independent TOML decoder; it is now a direct requirement. * fix(mtproto): force stderr logging and reject web listeners - Render forces logging.destination = "stderr": the node reads telemt's stderr for its log stream and startup errors, and a panel config with destination "file" or "syslog" would leave both empty. logging must be an object when present. - NewConfig rejects a server.listeners[].transport other than "mtproxy". telemt WEB listeners need [[web.vhosts]], which the node rejects, so they would only make telemt exit at startup. * fix(mtproto): supervise telemt 3.5.14 safely across start, crash and shutdown Process manager (process.go): - Refuse telemt older than 3.5.14 (parsed from `telemt X.Y.Z`). - Run `telemt run <config>` with no --pid-file, so a stale pid file can no longer block startup; reload sends SIGHUP to the child (Windows returns an error and the caller restarts). - Started() checks a waitDone channel and never reads ProcessState (data race under -race). Stop marks the run as stopping; any other exit is logged and published as "telemt exited unexpectedly: <err>", with an OOM hint on SIGKILL. - Read stdout and stderr to EOF before Wait so a crash's last lines are kept; overlong lines are cut instead of stalling the reader. - Keep the last 50 lines since Start for startup errors (StartupTail). - Kill an orphaned telemt whose command line runs exactly this config path before starting a new one. - Add Exited(), Generation(), Close(); Start after Close fails. Lifecycle (lifecycle.go): - New picks the API and metrics ports again while they clash with each other or with the config's listen ports, creates <generated>/telemt with mode 0700 and resolves symlinks (telemt 3.5.8+ rejects a symlinked config path). - Wait up to 90 s for /v1/health (telemt binds its API only after the Telegram DC check), then fail with telemt's recent output instead of a bare exit status. /v1/health/ready gets a 30 s budget and only logs a warning. - A supervisor restarts telemt after an unexpected exit with backoff from 1 s doubling to 60 s, re-rendering the desired users. - syncMu serializes every mutating call, the supervisor and Shutdown. Shutdown sets closed, stops telemt, closes the log buffer and deletes config.toml; later syncs return errClosed instead of starting a new telemt and crashing the node on a closed log channel. Also bump TELEMT_VERSION to 3.5.14 and give the integration tests a 150 s context. Unit tests drive a fake telemt (the test binary re-run as a helper process) for slow APIs, crashes, orphans and startup failures. * fix(mtproto): detach telemt startup from request deadlines - New starts telemt on context.WithoutCancel(ctx) bounded by telemtStartupTimeout, so a telemt that binds its API after the panel's 10 s Start deadline still comes up and the panel's retry attaches to it. Once spawned, startLocked waits for health on a detached ctx too; only the startup timeout and Shutdown (errClosed) end that wait. - The /v1/health/ready probe moved to a background watcher bound to the backend lifetime and to that telemt's exit: it warns after 30 s and logs when telemt becomes ready later. Start and every restart return right after health instead of holding syncMu and the panel deadline. - A sync whose ctx is already done (it queued on syncMu past its deadline) returns ctx.Err() before touching the config; reloadLocked never restarts telemt when the caller gave up, and startLocked spawns nothing for a done ctx or after Shutdown. - The supervisor and readiness watchers share one WaitGroup that Shutdown waits on. * fix(mtproto): apply user changes without per-user reads and confirm reloads User changes are now decided from the confirmed desired state, never with a GET per user. Unchanged users are skipped; up to 32 changes go through telemt's user API (PATCH falls back to POST on 404, POST to PATCH on 409 user_exists, DELETE of a missing user is done). Larger batches, changes to or from zero users, a failed partial apply and every full sync render the config once and reload it through POST /v1/system/reload (instant, rollback), polling the reload status for the outcome and retrying while another reload is in progress. desired changes only after telemt confirmed the new users. A rolled_back, failed or refused reload re-renders the previous users to disk and returns telemt's error. A reload whose caller gave up leaves telemt running and forces the next sync to apply all users. Only a reload request that cannot reach telemt restarts it. The online IP list uses one GET /v1/stats/users/active-ips call. The SIGHUP reload and waitForRuntimeUsers are removed, and DELETE responses (a JSON string) decode again instead of failing into the full-render fallback. * fix(mtproto): confirm hot reloads with a marker user instead of the reload API telemt's API reload swaps the runtime generation and, in instant mode, stops every session of the old one. A hot reload (SIGHUP, or telemt's config file watcher) updates the users live and cancels only changed or removed ones, so full applies go back to it. Every render now carries one disabled marker user with a fresh random name (pgm_<16 hex>). After writing the config the node sends SIGHUP (on Windows it relies on telemt's file watcher) and polls GET /v1/users until the marker is in telemt's running config, for up to 30 s, detached from the caller and ended by Shutdown. A config telemt did not load by then is rejected: the previous users are rendered again with a new marker and the error carries telemt's last output. A caller that gives up leaves telemt running and forces the next sync to apply all users; only an API that gets no answer restarts telemt. The marker replaces the zero-user sentinel and keeps telemt's user list non-empty, so the last real user is deleted, and the first one created, through the user API. The reload API client code is removed. * fix(mtproto): settle hot reloads independently of callers and clear stale overrides telemt stages an admission override for every user API write and clears it only when a published config carries the same value. A full render that changed such a user again before telemt published the API's own write left the user refused while the node reported success. The node now records users written through the API since the last confirmed render or telemt start, and before a render writes again each one whose rendered state differs, so the render's publish clears the override. A write that fails stays unknown and keeps the state unconfirmed. A full apply now runs to telemt's verdict in the background under syncMu, which a context-aware lock lets callers wait for only until their own deadline. A caller that gives up gets its ctx error at once; the apply still confirms, or restores the previous users, and logs the outcome. A rejection goes to telemt's log stream, which the panel shows. A 5xx "failed to load config" from GET /v1/users is an immediate rejection with telemt's message. telemt is restarted only after 3 polls in a row get no answer or when it exited, not on one failed request. Reload code moves to reload.go and the per-user API writes to userapi.go. * fix(mtproto): account traffic per stat family without double counting - Give users, outbound and inbound stats their own baseline per user, so the panel's 10 s user poll and 30 s outbound poll no longer consume each other's traffic. - Track every user telemt reports, desired or not. A user missing from a snapshot keeps its counters, so a removed and re-added user is not billed again and its last interval is still reported. Absent users are dropped after 25 h (telemt forgets idle stats after 24 h), keeping the outbound and inbound traffic still pending. - Count from zero after a telemt restart (process generation) or a counter decrease, carrying the pending delta of every family. - Serialize refreshes with refreshMu and share a snapshot younger than 1 s, so concurrent polls never apply an older scrape after a newer one. A scrape that overlaps a telemt restart is discarded. - Read telemt 3.5.14's metric names, filter by name before parsing labels, and switch to GET /v1/users totals (reported as downlink) while telemt_telemetry_user_series_suppressed is set; a source switch reseeds without double counting. - Report distinct client IPs as the online count, drop marker users from every result, and log failed refreshes at most once a minute. - Move the stats endpoints into stats.go. * fix(mtproto): carry exact traffic across a stats source switch The reseed on a switch between per-user metrics and API totals dropped the bytes telemt counted between the last sample of one source and the first of the other, in every family. Both sources count the same telemt stats entry (total_octets is octets_from_client + octets_to_client), so within one telemt process the increment is known exactly: credit it as downlink. A total below the last sample means telemt dropped the entry in between, which counts from zero like a restart. The suppression trigger also covers more than 4096 configured users; say so in the comments and the switch log line. * build(mtproto): ship pinned telemt 3.5.14 in the node image - Dockerfile: the builder runs make install_telemt after install_xray, and the final stage carries /usr/local/bin/telemt plus the telemt license files in /usr/share/licenses/telemt/. - Makefile: install_telemt checks the tarball against our own SHA256 per arch (x86_64, aarch64) instead of the release's .sha256 file, refuses "latest", and needs TELEMT_SHA256 for any version other than the pinned one. TELEMT_INSTALL_DIR selects the target dir; sudo is used only when that dir is not writable. --version runs only for a host-arch binary. - CI: install telemt and point MTPROTO_TELEMT_BIN at it, so the gated MTProto integration tests run. - third_party/telemt: telemt 3.5.14 LICENSE and the canonical English license text, copied verbatim, with a note on the unmodified binary. - README: MTProto backend, Xray and MTProto on one host, third-party note. * fix(mtproto): read telemt 3.5.14's answer to a user delete telemt 3.5.14 answers DELETE /v1/users/{name} with an object, {"username": ..., "in_runtime": ...} (status 202 while the running config still has the user), not with the username as a JSON string. The node failed to decode every successful delete, logged the batch as failed and fell back to a full render and hot reload for each user removal. The fake telemt API now answers deletes like telemt 3.5.14, and a client test pins the answer. Found by the live tests against telemt 3.5.14. * fix(mtproto): reject a render telemt cannot read without waiting 30 s telemt reads the config file and its includes before it parses them, and answers GET /v1/users with 500 "failed to read config graph: ..." when that read fails, for example on a rendered file above its 8 MiB limit per config file. The node only took "failed to load config" (a parse error) as a rejection, so a read failure kept the sync waiting the whole markerConfirmTimeout under the sync lock before the restore. Both answers now reject the render at once with telemt's message. Found by the live tests against telemt 3.5.14. * fix(mtproto): do not SIGHUP telemt before it handles the signal telemt installs its SIGHUP handler in its config watcher, which waits for the runtime to activate after the API is up, reads the whole config and sets up its file watches first. A SIGHUP in that window ends telemt with the default action. The node assumed the opposite order, so a sync right after a start (or a restore after a quick rejection) could kill telemt: the reload then found the API gone, restarted telemt and dropped every session. For telemtSignalGrace (5 s) after telemt's API first answers, the node now sends no SIGHUP. The watcher loads most renders of that time by itself, but misses one written while it sets up its watches, so a reload whose marker telemt has not loaded when the grace ends gets its SIGHUP then. The fake telemt can now play the watcher, including that blind spot, and a late SIGHUP handler. Found by the live tests against telemt 3.5.14, where nine telemts started at once made the window wide. * test(mtproto): cover telemt 3.5.14 lifecycle, reload and removal end to end The integration tests (run when MTPROTO_TELEMT_BIN names a telemt binary) now check what telemt does, not only what its config file says. A fake MTProto client performs the obfuscated2 handshake with a user's secret, and telemt's per-user connection gauge tells whether it admitted the client. telemt's dc_overrides point the client's DC at a local listener, so sessions stay open without Telegram. Covered live: start, version 3.5.14, health and stats, then Shutdown removing config.toml and leaving no telemt process; user create, secret change and delete through the user API; full syncs down to the marker alone and back on the same process, with the watcher and with SIGHUP; unchanged users keeping their sessions across a hot reload while removed users lose theirs; API deletes cutting sessions, including the last real user; a start without users; a secret written through the API and changed by a render before telemt published it; a symlinked generated path; and a render above telemt's 8 MiB limit, rejected at once with telemt's reason while the previous users and their sessions stay. The tests run in parallel, each with its own telemt. * refactor(mtproto): keep mtproto.go to the backend struct and drop dead code mtproto.go now holds the MTProto struct and its one-line accessors. The helpers it collected move next to their callers: Restart and the API token to lifecycle.go, the marker user to reload.go, the atomic file write and user map clone to sync.go. markerUsername, read only by tests, moves to the tests, and the unused config field goes. Also removes the never-built tomlLiteral type, the always-nil error of classifyTOMLSection, and decoded telemt API fields the node never reads. No behavior change. * chore(mtproto): fix the Windows test build and document telemt limits - Build integration_test.go only off Windows: it uses waitFor from the Unix-only fake telemt helpers, so GOOS=windows go vet failed. - Move the TestUsersAPIUnreachableRestartsTelemt doc comment back above its test. - README: Start can take up to 90 s and the panel's retry attaches; above 4096 users telemt stops per-user series and the node bills API totals as downlink; telemt refuses configs above 8 MiB (about 150k to 190k users). * fix(mtproto): send the first SIGHUP once telemt handles it telemt's config watcher installs its SIGHUP handler at the end of its own setup, which runs concurrently with the API coming up and can finish after the API first answers; a SIGHUP before that ends telemt, and once installed the handler stays for the process lifetime. On live telemt 3.5.14 the handler appeared about 3 ms after the API first answered, but a slow STUN probe, TLS front or large config can stretch that past any fixed grace. On Linux the node now sends SIGHUP only once bit 0 of the SigCgt mask in /proc/<pid>/status is set, checked on each marker poll and before the restore render's signal. The 5 s grace after the first API answer stays as the fallback on other platforms or when /proc cannot be read. The fake telemt now models the mask too: with a signal delay it resets SIGHUP to its default action until signal.Notify, since the Go runtime otherwise catches SIGHUP from the start. The grace test forces the fallback, a new Linux test leaves only the mask to let the signal through, and the live tests wait for the handler instead of the clock. * fix(mtproto): restart telemt whose API hangs during a reload A telemt whose API accepts connections but never answers was diagnosed as a rejected reload: each users poll ran into the 10 s client timeout, so the 30 s marker budget ran out before three unanswered polls, the node restored the previous render and reported a rejection, and nothing restarted telemt. Each marker poll now gets its own timeout, a quarter of the budget (7.5 s), so three hung polls in a row reach the existing restart branch at about 22.5 s. A shorter timeout would misread a large node: telemt reads its whole config for each poll, and listing 150k users took about 2.5 s on live telemt 3.5.14. * test(mtproto): cover Shutdown during a full apply in the marker wait A full apply blocked waiting for telemt to load its render must not hold Shutdown for the 30 s marker budget. The test starts a sync against a fake telemt that never loads the render, waits until the marker wait polls GET /v1/users (the fake now logs those requests), and calls Shutdown: Shutdown returns promptly, the apply ends with errClosed, needsFullApply is set on the closed backend, desired keeps the confirmed users and telemt is not restarted. * test(controller): cover a slow MTProto Start outliving its caller Dev now runs every core start under a context that keeps the request's values but not its cancellation (#116), and a same-config Start is a heartbeat only when the running backend passes GetSysStats (#117). The panel relies on both for MTProto: telemt may need up to 90 s to bind its API, the panel gives up on Start after 10 s, and its retry must attach to the telemt that came up instead of restarting it. Run the controller against a fake telemt (the test binary re-executed through a wrapper script, so the node starts and reaps a real child) that binds its API after a delay: a Start whose caller already went away ends with telemt running, telemt keeps running after the start context ends, and a retry that waits behind the start attaches to the same backend. * fix(mtproto): let Disconnect abort a telemt start in progress New detached the telemt start from its context (WithoutCancel plus its own 90 s bound) so a slow start would outlive the panel's 10 s Start request. Dev's controller now does that for every backend (#116): it runs StartOrAttach under a context without the request's cancellation and cancels it only from Disconnect, which process shutdown calls without LockControl, so it does not wait for a start in progress. The second detachment in New and startLocked ignored that abort. A Disconnect during a telemt start that never binds its API (Telegram unreachable) blocked on the controller mutex for the whole 90 s telemtStartupTimeout, and the telemt being started kept running until then. New and startLocked now follow their context; waitForHealth still bounds the wait by telemtStartupTimeout and reports Shutdown as errClosed. Every other start already passes a context the caller cannot cancel: Restart and the supervisor the backend lifetime, a full apply its detached apply context, the restart after a hung reload a WithoutCancel context. The panel sees no change: the Start outlives its request through the controller, as the controller tests show. * test(controller): restart telemt when an MTProto Start changes the config Dev's StartOrAttach now offers a changed config to a running backend that implements backend.LiveConfigApplier before it restarts it (#105). Only xray does: telemt has no live path for its core config, so a Start with another MTProto config must still stop the old telemt and start a new one on the new config. Cover that against the fake telemt, and fail loudly if MTProto ever gains the interface without a review of what a live apply means for telemt. * test: pin the node answers the panel relies on for MTProto The panel reads three answers of the node as fixed: - "invalid backend type" from a node without the MTProto backend, which it turns into an upgrade message; - "the users are still being applied" and "context deadline exceeded" from a sync that is still applying, which it retries later; - a retry or repair Start that attaches to a running telemt, which since #117 needs GetSysStats to answer within the 3 s heartbeat budget. Assert the exact texts, check that the unknown backend type error stays unwrapped under the detached start (also when the request is gone), and check that GetSysStats answers while telemt is frozen (SIGSTOP, so its API cannot answer) and syncMu is held: it stays a local read of the process, so a stuck API or a full apply never turns a heartbeat into a restart. * fix(mtproto): mark sync errors of a busy or restarting telemt as retry-later Bridge 0.9.4 delivers user deltas through SyncUsersChunked and treats an INTERNAL "failed to update users" answer (REST: 500) as a rejection of the users it sent: it splits the batch in halves, sends again and holds each user it isolates with a backoff of 2 s up to 60 s (an add every 300 s after 10 minutes). MTProto answered that way for failures that have nothing to do with the users: the backend shut down while a request held it, telemt that does not start for a full apply, a reload whose restart failed, a caller that gave up while it queued behind an apply. A busy or restarting telemt therefore multiplied the requests and delayed every user of the batch. Add backend.RetryLaterError, a marker that keeps the error and its text, and wrap every MTProto sync failure in it except a render telemt read and refused (errReloadRejected): the users sent may have caused that one, so splitting the batch can find them. An invalid user synced alone stays a plain error too. The texts the panel matches ("the users are still being applied", "context deadline exceeded") are unchanged. Xray and WireGuard never return the marker. * fix(controller): answer retry-later sync errors as unavailable A sync error a backend marked retry-later (only MTProto does: telemt is busy with an earlier apply, restarting or shut down) now reaches the client as gRPC Unavailable or HTTP 503 instead of INTERNAL (SyncUser, SyncUsersChunked), Unknown (SyncUsers) or 500 (REST), with the same text. Bridge 0.9.4 retries such an answer as a whole batch with its 1 s to 30 s backoff, the same path a timed-out sync already takes, instead of splitting the batch and holding each user it isolates. The texts the panel matches ("the users are still being applied", "context deadline exceeded") stay in the answer, so the panel's manual sync keeps reporting a still-applying MTProto node as retry-later. Every other error keeps its code, so Xray and WireGuard answers do not change. Tests: the mapping in both services, and the rpc and rest handlers end to end against an MTProto backend on the fake telemt whose full apply is still waiting for telemt when the caller's deadline passes (the fake now catches SIGHUP like telemt). * refactor(mtproto): render the telemt config with go-toml/v2 Replace the hand-written TOML encoder with github.com/pelletier/go-toml/v2, as asked in review. The test-only go-toml v1 dependency goes away. - JSON numbers from the panel config are converted to int64 or float64 first: the encoder writes a json.Number as a string. An integer outside the TOML 64-bit range is an error naming the field. - JSON nulls are still dropped from tables; a null array element is an error that names its path. - telemt expands any line "include = <path>" as a file include before it parses TOML, in every table. The old encoder quoted such keys; go-toml writes them bare, so the rendered text is now checked with telemt's own line matcher and refused, and the username "include" is skipped like any other invalid user. - Test helpers and the fake telemts decode the config with go-toml instead of parsing double-quoted lines by hand. * refactor(mtproto): use omitzero for the telemt user requests The create request now sends the limits as plain uint32 with omitzero instead of *int with omitempty, so the pointer and the int range conversion go away; a zero limit is still left out. The patch request keeps pointers without omitzero on the clearable fields: JSON Merge Patch needs nil sent as null to clear an override. * fix(mtproto): reject include keys at any depth of the panel config A key named include in any table becomes a telemt file include, and the render already refuses it; NewConfig now rejects it too, naming the path, so the panel gets the error up front instead of a failed Start. Also from review: - the TOML conversion refuses container types it does not walk, since they could hide a json.Number that would render as a string - a golden test pins one full render, so a go-toml upgrade that changes the output shows up - table test for checkNoIncludeLines against telemt's matcher, signed zero, and the request JSON without a secret - parseConfigUsers fails loudly when a config does not decode * feat(mtproto): accept telemt WEB listeners and vhosts behind a private front Accept the web section and listeners with transport "web" on a loopback or private address and their own port. Vhosts may only use an http_upstream decoy on a loopback or private IP literal, must not define profiles, and may carry a user_profile template that the node keeps per vhost and strips from the config. web.debug and web.limits.max_profiles stay node managed. * feat(mtproto): render one telemt WEB profile per user * fix(mtproto): apply user changes by render when WEB profiles are in use * fix(mtproto): render web.limits.max_profiles within telemt's memory envelope telemt 3.5.14 reserves 64 bytes of its WEB memory envelope per max_profiles entry, and its default envelope (1280 MiB) has only about 4.1 MiB left after its other default reservations. With 262144 profiles (16 MiB) every WEB config failed at start with "web.limits memory reservations must fit memory_envelope_bytes within 4 GiB". Render 65536 (4 MiB), which telemt accepts with its default limits. A render with more profiles than that (users times vhosts) is rejected by telemt and the node restores the previous users, as for any rejected render. * test(mtproto): cover WEB decoy, profiles and empty start against telemt 3.5.14 startLiveTelemtWithConfig lets a live test pass its own panel config; startLiveTelemt keeps today's config through it. TestIntegrationWebDecoyAndProfiles runs a native and a WEB listener on 127.0.0.1 with one vhost and a local decoy: a plain request for the vhost reaches the decoy, GET /v1/config lists a profile per user and follows a full sync, and a native session stays open across that sync. TestIntegrationWebStartWithoutUsers starts with no real users: telemt accepts the disabled marker user's profile, so no fallback is needed. * test(mtproto): check WEB profiles in telemt's running config GET /v1/config reads telemt's config file on disk, so it showed only that the node rendered the profiles. The WEB tests now also send the bridge request Telegram Desktop sends for a dd link (capability HMAC-SHA256 over the context and host, keyed by the client secret) and check telemt's answer: the bridge page for a user with a running profile, the decoy once a sync removed it, and telemt's private 404 for the disabled marker, whose profile telemt runs but serves no bridge to. The /v1/config check stays, named as a check of the rendered file. * docs(mtproto): explain WEB mode and an example HTTPS front * docs(mtproto): add tls_domain to the WEB example config * fix(mtproto): match telemt's decoy origin rule and keep decoys off telemt's own ports A WEB decoy upstream must now be a bare http origin (no https, path, query, fragment, credentials or port 0), as telemt's parse_origin requires, so the node refuses it before telemt fails at Start. Render also refuses a loopback decoy on the node's telemt API or metrics port, which would publish that endpoint on the WEB domain. Also fix the max_profiles rejection reason and the UpdateUsers comment, and say in the README that web_trusted_proxy_cidrs is required and where telemt's per-profile limits stop. * fix(mtproto): check WEB profile totals and base paths before telemt does telemt counts WEB profiles across all vhosts and refuses a config above web.limits.max_profiles, and it refuses a vhost base_path that is not empty or slash-separated [A-Za-z0-9][A-Za-z0-9_-]* segments of at most 128 ASCII bytes. Render now refuses more than 65536 profiles before writing the file, naming the user and vhost counts, and NewConfig applies telemt's base_path rule. The README notes that `http2 on;` needs nginx 1.25.1 or newer. --------- Co-authored-by: x0sina <bulletsina77@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem with Start:
default_timeout(10 s by default), while a 163k-user start takes about 13 s even on a fast host.Problem with full sync: a full
SyncUsersthat timed out restarted Xray a second time on the old config and dropped the new users.Change:
StartOrAttachruns under a context without the request's cancellation, bounded by a 3 minute start timeout.Disconnect(process shutdown) still aborts a start in progress.Shutdownstill ends that wait.Tests:
Disconnectaborts the start).-1, thenstarted=False, and every retry fails.-1, then the attach probe seesstarted=Trueafter 2.5 s, and the retried Start returns in 0.8 s on the same Xray.No wire change.
Known limitation (N3): the controller mutex is held for the whole start. Every RPC, including auth and
Stop, waits until the core is up or the 3 minute start timeout passes; before this change the wait ended at the caller's timeout. The panel's attach probe after a timed-out Start depends on this wait. A follow-up can build the backend outsidemu, keepGetBaseInfowaiting (bounded) for a start in progress, and letStopcallabortStart()beforeLockControl().A timed-out full sync (
SyncUsers) now applies the new users instead of restarting twice and reverting. So a-1on a full sync no longer means the users were reverted: the node is on the new users and the next health cycle marks it recovered.