Skip to content

cpu-profile: invoke node directly (skip pnpm/cross-env wrappers)#111

Merged
JohnMcLear merged 1 commit into
mainfrom
fix/cpu-profile-direct-node
May 16, 2026
Merged

cpu-profile: invoke node directly (skip pnpm/cross-env wrappers)#111
JohnMcLear merged 1 commit into
mainfrom
fix/cpu-profile-direct-node

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Summary

  • Previous workflow (feat(scaling-dive): CPU-profile capture workflow #109 + fix(cpu-profile): unique cpuprofile names + diagnostic logging #110) ran the SUT via pnpm run prod with NODE_OPTIONS=--cpu-prof. The pnpm → cross-env → tsx chain produced 5 cpuprofile files; pgrep matched the tsx wrapper, which came back 99.8% idle, while pnpm itself dominated (99.7% in spawnSync).
  • Invoke node --require tsx/cjs node/server.ts directly with NODE_ENV=production and --cpu-prof --cpu-prof-dir=... so there is exactly one Node process under profile, and capture its pid via \$! instead of pgrep.
  • Use that pid for SIGTERM at shutdown (with pkill fallback).

Continues the lever-8c CPU hotspot investigation for ether/etherpad#7756.

Test plan

  • After merge, run CPU profile capture workflow against develop.
  • Confirm only one .cpuprofile file lands, and that it contains > 80% busy samples with stack frames in src/node/....

The previous workflow exported NODE_OPTIONS=--cpu-prof and ran `pnpm run
prod`, producing 5 cpuprofile files (one per Node process in the
pnpm → cross-env → tsx chain). The pgrep-based SUT pid capture matched
the tsx wrapper rather than the actual server.ts process — analysis
of run 25956220071 showed pid 2636 (pnpm) blocked in spawnSync for
113s while the recorded sut pid 2684 (tsx) was 99.8% idle.

Invoke node directly with the same args the prod script uses
(`node --require tsx/cjs node/server.ts`) under --cpu-prof, set
NODE_ENV inline, and record the SUT pid via `$!` so there's exactly
one process to profile and stop. The earlier --cpu-prof-name pid-suffix
fix from PR #110 is retained.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@JohnMcLear
JohnMcLear merged commit 66d03d2 into main May 16, 2026
4 checks passed
@JohnMcLear
JohnMcLear deleted the fix/cpu-profile-direct-node branch May 16, 2026 07:39
JohnMcLear added a commit that referenced this pull request Jun 21, 2026
…ient 4.0.3 + rate-limit) (#122)

* build(deps): bump actions/checkout from 4 to 7

Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 7.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v7)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix(ci): restore sweep integration test against hardened core

The sweep integration test (and thus all Backend-tests CI) has been red on
every PR since ~2026-05-19; the last green run was #111 (2026-05-16). The
break is not in this repo — ether/etherpad#7773 ("harden: reject USER_CHANGES
inserts without an author attribute", merged 2026-05-16 17:23, just after the
last green run) tightened server-side changeset validation and bumped its own
etherpad-cli-client dependency to 4.0.3 to comply.

This repo was still pinned to etherpad-cli-client 4.0.2, whose append() splices
at text.length (stranding the trailing '\n') with an empty apool and no author
attribute — exactly the two shapes #7773 now rejects as badChangeset. Result:
zero ACCEPT_COMMITs, so every step's latency sample count is 0 and the test
asserts `count > 0`.

Two-part fix, both verified against a local core build at develop HEAD:
- Bump etherpad-cli-client 4.0.2 -> 4.0.3 (sends author-attributed inserts and
  preserves the trailing newline).
- Raise commitRateLimiting on the SUT in the sweep step. The sweep drives 2..4
  authors from a single IP; core's default 10 changes/s/IP rate-limits the
  4-author step (~20/s), and a rate-limited USER_CHANGES never gets an
  ACCEPT_COMMIT, stalling the cli-client's in-flight slot. Mirrors the
  importExportRateLimiting bump runnerLoadTest.sh already applies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant