Repository navigation
⏱️ fix: Stop the Event-Loop Delay Monitor From Waking the Idle Server 100 Times a Second - #199
Merged
Merged
Conversation
… 100 Times a Second
collectDefaultMetrics() ran with prom-client's default
eventLoopMonitoringPrecision of 10 ms. That is a monitorEventLoopDelay
histogram sampling every 10 ms, so Bun's main thread woke about 100 times a
second with no traffic. The panel now samples once a second, which keeps the
nodejs_eventloop_lag_{min,max,mean,stddev,p50,p90,p99}_seconds metrics.
ADMIN_PANEL_METRICS_EVENT_LOOP_PRECISION_MS overrides the interval and is
documented in .env.example. A value that isn't a whole number from 1 to
2147483647 falls back to 1000: monitorEventLoopDelay throws for anything else
(Bun's ceiling is 2147483647), and prom-client would rethrow it at startup.
Idle main-thread wakeups of `bun server.ts` on a production build went from
about 100 a second to 1 a second on Bun 1.4.0 and 1.4.3.
Refs #97
Contributor
Author
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
# Conflicts: # src/server/metrics.test.ts
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.
Pull Request Template
Summary
With no traffic,
bun server.tswoke its main thread about 100 times a second. The cause wasclient.collectDefaultMetrics()insrc/server/metrics.ts. prom-client 15 defaultseventLoopMonitoringPrecisionto 10 ms and passes it as the resolution of aperf_hooks.monitorEventLoopDelayhistogram. prom-client skips that histogram only when the runtime reports it as not implemented, and Bun 1.4.0 and 1.4.3 implement it. So the server ran a 10 ms timer on its main thread for as long as it was up.The panel now passes
eventLoopMonitoringPrecision: 1000.nodejs_eventloop_lag_{min,max,mean,stddev,p50,p90,p99}_seconds, now from one sample a second instead of 100.nodejs_eventloop_lag_secondsis measured when Prometheus scrapes and doesn't change.ADMIN_PANEL_METRICS_EVENT_LOOP_PRECISION_MSoverrides the interval for deployments that want the finer percentiles back. It is documented in.env.example.monitorEventLoopDelaythrows for any other value (2147483647 is Bun's ceiling, lower than Node's), and prom-client rethrows everything except "not implemented", so a bad value would otherwise stop the server at startup.collectDefaultMetricsinmetrics.ts, still run when the module loads, so the tests can run it against a fresh registry.About #97. This removes the server's only steady source of idle wakeups, but this PR refers to #97 instead of closing it. The issue reports about 8% of a core and an active timerfd re-arming every second. Neither reproduced here:
/proc/<pid>/statmdescriptor the issue points to is Bun's own. Bun 1.4.0 keeps it open for a bareBun.servewith no app code.setTimeoutis a short wait before a save is read back, inside the save request. A 10-secondsetIntervallike the memory tick proposed in 🪦 feat: Request Tombstones, Memory Watermarks & Server Error Logging #102 would add about 0.1 wakeups a second, which doesn't show up in these counts.If the reporter's container still idles near 8% with this change, the one-second timer comes from something this host doesn't have.
Refs #97
Change Type
Testing
Unit tests in
src/server/metrics.test.tsrun prom-client's realcollectDefaultMetricsagainst a fresh registry. A spy onperf_hooks.monitorEventLoopDelaycalls through, records the resolution and then disables the histogram. The tests check that:ADMIN_PANEL_METRICS_EVENT_LOOP_PRECISION_MS=250gives 250;"",0,-5,1.5,abcand2147483648fall back to 1000;/metricsserves hasnodejs_eventloop_lag_p99_seconds.With prom-client's default restored, every precision test fails because the monitor gets 10. Without the upper bound, the
2147483648case fails. The registry test fails if the module-level call is dropped.Idle measurement: a production build (
bun run build) served withbun server.tsandNODE_ENV=production, with no traffic after one readiness request. Measured 60 seconds after start over three 20-second windows, counting the main thread's voluntary context switches from/proc/<pid>/status:mainOn this PR's build, the first window after start or after a few requests can show 6 to 13 wakeups/s while Bun's worker threads wind down. Counts across all threads aren't in the table because those threads start and exit during a window.
Live checks on this PR's build:
ADMIN_PANEL_METRICS_EVENT_LOOP_PRECISION_MS=10it goes back to 99 to 111 wakeups/s, so the variable reaches the monitor under Bun.2147483648the server starts and serves the login page. prom-client on its own, given that value, throws at startup under Bun./metricswith the bearer token still serves all seven histogram gauges,nodejs_eventloop_lag_seconds, the process metrics and theadmin_http_*counters. Without a token it answers 401.Playwright e2e wasn't run because it serves the panel with
vite dev, which never loadsserver.tsorsrc/server/metrics.ts.Test Configuration:
dev, MongoDB replica set, for the click-through.npx tsc --noEmit,npx eslint src/ e2e/ --max-warnings 0,npx prettier --checkon the changed files,bun run test:lint-rules,npx vitest run(3347 tests),bun run build.Checklist