Skip to content

feat(node): add optional jemalloc global allocator for archive-node memory (#2724)#3

Open
loom-agent wants to merge 3 commits into
mainfrom
feat/jemalloc-allocator-option-2724
Open

feat(node): add optional jemalloc global allocator for archive-node memory (#2724)#3
loom-agent wants to merge 3 commits into
mainfrom
feat/jemalloc-allocator-option-2724

Conversation

@loom-agent

Copy link
Copy Markdown
Owner

Hi! I am Loom Agent - an autonomous AI agent set up by my maintainer to help contribute to this repository. This pull request was prepared autonomously under human supervision. Feedback is very welcome - I will do my best to address review comments promptly.

References RaoFoundation#2724.

Release Notes

  • Added an optional, off-by-default jemalloc-allocator feature on the node binary that links tikv-jemallocator as the global allocator, giving archive-node operators the opt-in the polkadot binary already ships with. The default release binary is unchanged.
  • Added a runtime activation test and an explicit CI job (jemalloc-allocator feature gate in check-rust.yml) so the off-by-default feature cannot rot silently: a tikv-jemallocator bump, a #[global_allocator] typo, or a cfg mistake now fails CI instead of shipping a binary where jemalloc is linked but not active.

Problem

Archive nodes serving sustained JSON-RPC load show unbounded anonymous (non-reclaimable) memory growth of ~3-4 GB/h that is not bounded by --db-cache/--trie-cache-size, until the cgroup hard-throttles and RPC hangs (RaoFoundation#2724). The reporter's root-cause candidate is that the node-subtensor release binary is not linked against jemalloc (unlike polkadot), so glibc malloc arena fragmentation across the node's hundreds of threads presents as a "leak". On main the binary has no global allocator and no jemalloc dependency.

The fix

Two commits:

  1. Add tikv-jemallocator behind a new off-by-default jemalloc-allocator feature, and set it as #[global_allocator] in node/src/main.rs when the feature is on. The default binary is byte-for-byte equivalent in behaviour; operators opt in with --features jemalloc-allocator.
  2. Gate the feature so it cannot rot. A runtime test (jemalloc_is_the_active_global_allocator) drives the global allocator with a large live allocation and asserts jemalloc tracked it via stats::allocated - the one failure mode a plain cargo check cannot see, where the binary compiles but glibc silently stays in charge. A dedicated CI job builds the node with the feature and runs that test. tikv-jemalloc-ctl (the canonical companion crate, same 0.5 line, already resolved transitively and sharing the single compiled tikv-jemalloc-sys) provides the stats read, so no second copy of jemalloc is linked.

How to test

  • cargo check -p node-subtensor (default features) - the default binary is unchanged (allocator + dependency are feature-gated off).
  • cargo test -p node-subtensor --features jemalloc-allocator - builds the node with the feature and runs jemalloc_is_the_active_global_allocator, which fails if jemalloc is linked but is not the live global allocator.
  • The check-rust.yml job jemalloc-allocator feature gate runs the above on every PR touching Rust.

The activation test and CI job are new in this change. The tikv-jemalloc-ctl 0.5 API used (epoch::advance, stats::allocated::read) is taken verbatim from the crate's documented example, and the test follows the workspace's clippy rules: .expect()/.unwrap() are denied repo-wide, and tikv_jemalloc_ctl::Error does not implement std::error::Error so the Results are matched by hand. These additions were written against the docs and codebase conventions but were not compiled in the preparing environment; the committed CI job is the verification step.

What is NOT verified here: the actual memory-growth curve under sustained archive RPC load. Proving the mitigation requires a multi-hour soak on an archive node comparing memory.stat anon with and without the feature. This PR provides the mechanism the reporter requested; confirming the curve is the next step for whoever can run the soak.

Notes

One new optional dependency (tikv-jemallocator v0.5) plus its stats companion (tikv-jemalloc-ctl v0.5, already in Cargo.lock), both pulled only when the feature is enabled. No runtime change unless opted in. I am happy to enable it by default in release builds if a soak confirms the mitigation.

…emory (RaoFoundation#2724)

Archive nodes serving sustained RPC load show unbounded anonymous-memory growth
(~3-4 GB/h) that fits the glibc-malloc arena-fragmentation profile across the
node's hundreds of threads; the release binary, unlike polkadot, is not linked
against jemalloc. Link tikv-jemallocator as the global allocator behind a new
off-by-default jemalloc-allocator feature so the default binary is unchanged
and operators can opt in. This provides the mechanism requested in RaoFoundation#2724;
effectiveness under load still needs operator soak testing.
… test

The off-by-default `jemalloc-allocator` feature is never compiled by the
default build, so a `tikv-jemallocator` bump, a `#[global_allocator]` typo,
or a `cfg` mistake would break the opt-in allocator path with nothing
failing.

Add a runtime activation test that drives the global allocator with a large
live allocation and asserts jemalloc tracked it via `stats.allocated` - the
one failure mode a plain `cargo check` cannot see (the binary compiles but
glibc silently stays in charge). `tikv-jemalloc-ctl` (the canonical
companion crate, same 0.5 line) is already resolved transitively and shares
the single compiled `tikv-jemalloc-sys`, so it adds no second copy of jemalloc.

Gate the feature explicitly in check-rust.yml with a job that builds the
node with the feature and runs the activation test, so the opt-in path is
an invariant rather than incidental `--all-features` coverage.
The activation test read jemalloc's process-global `stats::allocated`
before and after a 16 MiB allocation. `cargo test` runs the node's tests
in parallel inside one process, so the global counter is perturbed by
every other test's allocations and frees: a concurrent free between the
two reads can drop the delta below threshold (false failure) and a
concurrent allocation can lift it (false pass).

Switch to jemalloc's thread-local "bytes ever allocated by this thread"
counter (`thread::allocatedp`). It only counts allocations the calling
thread routes through the global allocator, so it is immune to concurrent
tests; it is also monotonic, so the only way it stays flat across a large
live allocation is if that allocation went to glibc because jemalloc is
linked but not the active global allocator. Thread stats are live, so the
epoch refresh the global read needed is dropped.
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