* Initial memory tracking design doc draft, plus first round of review comments by me with // TODO annotations
* design/memory-tracker: address first-round review TODOs
Resolves all // TODO annotations from the initial draft:
- live-block table now optional via MEMORY_TRACKING_LIVE_TRACKING knob
- drop the mmap slab pool; std::malloc + in-tracker flag is sufficient
- add MEMORY_TRACKING_FORCE_SAMPLE_BYTES so large allocations are always
captured regardless of the count-rate sampler
- add live block / byte totals to MemoryTrackerSummary
- replace the manual coverage spot-check with a sentinel-function unit
test that introspects the aggregation table directly
- leave ALLOC_INSTRUMENTATION alone; new hooks sit next to (not
replacing) existing conditional ones
- drop SJLJ jargon, trim A4 alternative now that force-sample-large
collapses the byte-rate-vs-count-rate question
* flow: add sampled per-call-site memory tracker
Adds a sampled memory attribution layer (flow/MemoryTracker.{h,cpp})
hooked into the three primary allocation paths — global operator
new/delete, FastAllocator, and ArenaBlock::create — plus a periodic
TraceEvent dump driven from SystemMonitor. Knobs gate sample rate,
force-sample threshold, report cadence, top-N, and capture depth;
prod default is off. See design/memory-tracker.md.
Test fixes uncovered while bringing the unit tests up:
- memTrackerResetForTest now resets the per-thread sample counter and
force-sample threshold. Without this, a test that exercised the
off-switch path left gMemTrackerCounter at INT_MAX, which then
silently suppressed sampling for the remainder of the run.
- Slow-path reseed special-cases inverse==1 to keep the counter at 1.
The general formula 1 + r % (2*inverse) yields counter values 1 or
2 at inverse==1, sampling only ~67% of allocations rather than every
one, which broke a test that asserts exact alloc counts.
- Sentinel functions in MemoryTrackerTest.cpp now route the allocated
pointer through an asm-volatile escape() helper. Clang -O3 was
eliding the new/delete pair (P0593 heap fusion), so the test's
allocations never reached the operator-new override.
* design/memory-tracker: address second-round review
- threshold-based reporting (80 MB default, ~1% of 8 GB target RSS)
replaces fixed top-N
- prod report interval 60 s -> 10 min; sim stays 30 s
- single combined MemoryTrackerAddrCmd event with one addr2line
invocation per dump (positional mapping back to sites), keeping
frame 0 — old design's per-site format_backtrace dropped the
leaf alloc frame
- new R12 "Side-thread coverage" + "Side-thread safety" subsection
documenting the FP-elision crash mode found via joshua repros
(RandomUnitTests / IThreadPool seeds segfaulted in captureStackFP
when walking from FastAllocator<N>::~ThreadData into glibc's
FP-elided pthread shutdown machinery) and the stack-bounds
mitigation via pthread_getattr_np
- R7 wording: live bytes (not cumulative)
- CallSite struct in design overview aligned with implementation;
ForceSampledCount promoted from prose-only to struct + emitted
detail; exemplarFrames sized to MEMORY_TRACKER_MAX_FRAMES (=10)
matching the FRAMES knob's stated 1-10 range
- LIVE_TRACKING=false degraded-mode interaction documented
- stale "slab pool" refs removed (std::malloc was already in code)
and fdbserver.cpp self-contradiction resolved
- R3 / Rollout reconciled (table shows steady-state, step 1 lands
at 0)
- 4-6 frame count flagged as initial estimate, subject to refinement
- file:line citations stripped from path references (line numbers
drift; symbol names are stable)
* flow: threshold-based memory tracker reporting + side-thread safety
Implements the second-round design changes in flow/MemoryTracker.{cpp,h},
flow/Knobs.{cpp,h}, flow/SystemMonitor.cpp.
- MEMORY_TRACKING_TOP_N -> MEMORY_TRACKING_REPORT_BYTES_THRESHOLD
(int64_t, default 80,000,000). MEMORY_TRACKING_REPORT_INTERVAL prod
default 60.0 -> 600.0; sim still 30.0.
- memTrackerDump(int topN) -> memTrackerDump(int64_t bytesThreshold).
Filters by liveBytes (or cumulativeBytes when LIVE_TRACKING=false)
>= threshold; emits MemoryTrackerSite per qualifying site plus one
MemoryTrackerAddrCmd event with a single addr2line invocation
covering every qualifying site's frames in dump order. The Summary
event picks up SitesReported and ReportBytesThreshold details.
- AddrCmd is built directly here (not via platform::format_backtrace,
which deliberately drops index 0 for its single-site use case); the
leaf alloc frame is preserved.
- captureFramesFP gains a per-thread stack-bounds check via
pthread_getattr_np + pthread_attr_getstack, cached in TLS. Without
it, walking the FP chain from FastAllocator<N>::~ThreadData into
glibc's FP-elided pthread shutdown machinery follows an
uninitialized saved-FP slot and dereferences garbage. Fixes the
joshua-found segfaults on RandomUnitTests seeds 3288611985,
3731245491, and 2219741568 (all in the IThreadPool worker-exit
path). See design/memory-tracker.md "Side-thread safety".
* flow/MemoryTracker: stub the FP walker on non-Linux
pthread_getattr_np is glibc-specific and the macOS build broke on
it. Frame-pointer walking on macOS is also unreliable on its own
(system runtime has -fomit-frame-pointer in places we can't
control), so a "loose bounds" workaround would still risk crashes.
FDB is required to compile on macOS but is not run in production
there. Gate initStackBoundsForThread + the real captureFramesFP
on __linux__; provide a return-0 stub on non-Linux. The rest of
the tracker (sample counters, aggregation, dump) still compiles
and runs; per-call-site reports on macOS will just lack stack
attribution.
* flow: clang-format fixup for memory-tracker files
Whitespace-only. Catches up flow/Arena.cpp and flow/MemoryTrackerTest.cpp
with the project's clang-format style; the original implementation
commit (d587f82b) slipped these past the format pre-flight.
* edit for clarity, brevity, and uniform voice
* flow/Arena: fix double-tracking on the >256/huge ArenaBlock paths
ArenaBlock::create's >256 and huge paths go through
allocateAndMaybeKeepalive (`new uint8_t[]`), which fires the global
operator new[] hook in addition to the explicit memTrackerOnAlloc
that fires immediately after. Two sites tracked the same pointer;
on free only the explicit-Arena fingerprint was debited, so the
operator new[] fingerprint accumulated liveBytes monotonically and
LiveBytesTotal/LiveBlocksTotal skewed by +n/+1 per arena alloc/free
pair. Reported as B1 in the PR review.
Fix at the Arena layer (so non-arena allocateAndMaybeKeepalive
callers in serialize.h's PacketBuffer code remain attributed at the
operator-new layer): a MemTrackerSuppress RAII helper held across
the underlying new[]/delete[] in ArenaBlock::create and
ArenaBlock::destroyLeaf.
Adds accounting tests for FastAllocator<32>, Arena small, Arena
medium (the B1 path), and Arena huge. The load-bearing assertion is
"exactly one site has the sentinel's frames AND nonzero bytes" --
fails pre-fix for medium/huge with sites=2. Tests gate on __linux__
since captureFramesFP is a no-op on macOS.
* flow/memory-tracker: address PR review follow-ups
- Knobs.cpp: sim default for MEMORY_TRACKING_REPORT_BYTES_THRESHOLD
drops 80 MB -> 1 MB so sim dumps surface more sites for manual
sanity-checking. Prod unchanged.
- B2: memTrackerForEachSite holds MemTrackerSuppress across the
callback loop so callbacks that allocate (e.g. fprintf failure
dumps) don't re-enter tracking under SAMPLE_INVERSE=1.
- B8: drop the always-zero g_reentrantBailouts and its
SamplesDroppedReentry summary detail. Wiring it would require an
atomic in the inline hot path, which R1 forbids. Design doc
updated.
- B3/B6/B7/B9: explanatory comments only -- frame-strip-count
inlining assumption (B3), unstable sort acceptable under R6 (B6),
shared xorshift seed acceptable in practice (B7),
MEMORY_TRACKING_LIVE_TRACKING is startup-only (B9).
* flow/MemoryTracker: one addr2line per site, drop chunking
Move the addr2line command back onto MemoryTrackerSite as a per-site
AddrCmd detail and remove the MemoryTrackerAddrCmd event entirely.
Each AddrCmd carries exactly that site's stack -- short, well under
the trace-detail truncation cap, ready to paste. Replaces the
consolidated-then-chunked-into-byte-buckets approach, which split
stacks across chunk boundaries and made raw events hard to read.
* add standard Apache 2.0 license headers to new memory-tracker files
The three new files (flow/MemoryTracker.{cpp,h} and
flow/MemoryTrackerTest.cpp) shipped without the project-standard
copyright/license block. Adds the standard 19-line header to each;
file-purpose comments stay below it, switched to // line comments
to keep the license block visually distinct.
AGENTS.md gains a short "Source File Headers" section so the next
contributor doesn't repeat the omission.
* address review comments; reduce cost of enable check; maintain net estimates so users dont have to do it manually
* formatting
* unit test bug fix
* gglass review comments on memory-tracker.md design doc
* design doc updates, and fill in a plan for remaining tests/benchmarks
* delete useless simulation section
* flow/bench: add memory-tracker microbenchmarks; record measured overhead
Add flow/bench/BenchMemoryTracker.cpp (Google Benchmark) measuring the tracker's
per-op cost via three benchmarks -- raw malloc/free baseline, end-to-end
operator new/delete, and isolated memTrackerOnAlloc/OnFree -- each at sample
inverse 0 / 100 / 1. Auto-picked up by the flow_bench CONFIGURE_DEPENDS glob;
run with:
bin/flow_bench --benchmark_filter=memtracker
Replace the placeholder "< 5% delta" targets in the design doc's Microbenchmarks
section with the measured numbers and a projected per-second overhead at an
assumed 100K alloc/sec. Headline: ~1.9 ns/pair disabled, ~5.6 ns/pair at the
production 1% rate (~0.056% of a core at 100K/s, ~1/17th of R0's 1% ceiling);
the every-allocation rows are labeled a buggified worst case, not a default.
Testing: built flow_bench on the dev pod (clean, -Werror) and ran the memtracker
filter six times; results stable to within a few percent across runs.
* design/memory-tracker: describe the coverage test as implemented, not proposed
The "Coverage spot-check via sentinel functions" section described the
already-implemented `coverage` test in proposal tense ("Add an introspection
API:", "The unit test:"), which read as future work. Reword to present tense
referencing the actual `coverage`/`*Accounting` tests and the existing
`memTrackerForEachSite` API. No remaining references to unwritten test cases.
* remove extraneous detail from requirements section
* substantially revise microbenchmark results based on seeing 2M allocations/frees on a CPU-maxed storage server
* add script to drive A/B experiment for sampled memory allocation tracking
* flow/MemoryTracker: move global operator new/delete into a server-only TU
The global operator new/delete replacements that route allocations through
the memory tracker lived in flow/MemoryTracker.cpp, i.e. in the flow static
library. flow is linked into libfdb_c and every client binding, so the
interposition shipped into client artifacts and could interpose the whole
host process's allocator even with sampling off.
Move them into a new fdbserver/GlobalNewDelete.cpp, compiled directly into
the fdbserver executable (which clients never link), mirroring where the
legacy ALLOC_INSTRUMENTATION overrides already lived. This also makes the
replaceable-symbol interposition reliable (guaranteed in the final link)
rather than dependent on static-archive pull-in. The legacy
ALLOC_INSTRUMENTATION overrides move into the same file, selected by
#if defined(ALLOC_INSTRUMENTATION) / #else, so exactly one set of global
operators is ever defined. No CMake change is needed (fdbserver/*.cpp is
globbed).
Also reconcile the design doc with the code: override placement, the Files
section (drop the unnecessary flow/CMakeLists.txt edit), the sim knob-table
values (REPORT_BYTES_THRESHOLD, SAMPLE_INVERSE prod default), the
FORCE_SAMPLE_BYTES "-1 disables" sentinel wording, and drop the stale
"buggify inverse to 1" note -- the every-allocation and sampled/weighted
paths are already pinned deterministically by MemoryTrackerTest.cpp.
Testing: build green (run-ccmk5); fdbserver -r unittests -f /flow/MemoryTracker/
runs all 10 memory-tracker unit tests, 0 failed.
* contrib/mako_ab_memtracker: disable RocksDB direct I/O for tmpfs runs
RocksDB opens its DB with O_DIRECT by default; /mnt/ram (tmpfs), where the
harness puts its data dir, does not support direct I/O, so the storage
engine fails to Open and the cluster never configures (fdbcli "configure
new" hangs, mako never starts). Pass the existing
ROCKSDB_USE_DIRECT_READS / ROCKSDB_USE_DIRECT_IO_FLUSH_COMPACTION knobs (=0)
for the rocksdb arm only -- no new knobs added. redwood is unaffected.
* fdbserver/bench: move memory-tracker microbench to fdbserver_bench
The global operator new/delete override lives in fdbserver/GlobalNewDelete.cpp
(server-only, for client isolation), so flow_bench -- which links only flow --
could not exercise it: bench_memtracker_operator_new hit libc++'s operator new
and its Arg(100)/Arg(1) rows were identical to Arg(0).
Move BenchMemoryTracker.cpp to a new fdbserver/bench/ whose CMake compiles
GlobalNewDelete.cpp into the fdbserver_bench executable (via ADDL_SRCS), so the
real override is a strong definition in the bench link and the operator-new
benchmark measures the actual hooked path. It links only flow, not the fdbserver
dependency graph. Adds a BenchMain.cpp (BENCHMARK_MAIN equivalent) and wires the
subdirectory into fdbserver/CMakeLists.txt.
Testing: fdbserver_bench builds; bench_memtracker_operator_new now shows distinct
off / 1% / every-alloc costs (11.4 / 14.4 / 80.9 ns), confirming the override
fires.
* design/memory-tracker: reconcile with code and slim down
Reconcile the doc with the implementation and trim implementation detail that
duplicated the code and had begun to drift:
- Fix drift found in review: degraded-mode live/peak fields stay 0 (not
"tracking the cumulatives"); the dump thresholds on the estimated fields (fix
the memTrackerDump header comment too); the live-block table is allocated but
empty when live-tracking is off (not "never allocated"); list the test file
and its forceLinkMemoryTrackerTests() wiring.
- Slim ~220 lines: replace the CallSite struct, the full captureStackFP source,
both TraceEvent .detail() schemas, and the file-by-file inventory with prose
that defers exact structs/keys/constants to the code; soften the knob table
to intent (authoritative defaults live in Knobs.cpp).
- Refresh the microbench numbers from fdbserver_bench and annotate the host
(AMD EPYC 9R14, clang -O3); replace the A/B placeholder with the measured
-16.5% (redwood) / -9.9% (rocksdb); note it is a point-in-time snapshot.
- Frame R0 as the target the v1 single-lock design does not yet meet (ships off
pending lock sharding); add a one-line note on table teardown/fork behavior.
* fix clang-tidy error
* mako wrapper scripts: take care to clobber the ramdisk before runs to avoid low-space throttling
* design/memory-tracker: note the two mako A/B harnesses and the off-state result
Point at contrib/mako_ab_memtracker.py (sampling off vs on) and
contrib/mako_ab_binaries.py (vanilla main vs this PR built with tracking off),
and record the latter's measured off-state overhead of -0.53% (redwood) /
-0.10% (rocksdb) on a shared base commit — well within R0's 1% ceiling.
* Add the default code review guidance to AGENTS.md
* address big brother review comments; add some braces and trim some generated comments (hard to believe, but true)
* clang-tidy again
* Final read-through of this PR.
-- Add or enhance a few comments on important items (performance & reliability)
-- Delete some misc agent-written comments, typically exhibiting recency bias e.g. naming bugs identified in code review passes or describing mundane earlier bugs
-- Add braces (InsertBraces style)
* memory-tracker: address round-4 review
Correctness/robustness:
- operator new now runs the installed std::new_handler retry loop, so an
allocation failure (including the tracker's own map growth) reaches FDB's
platform::outOfMemory / FDB_EXIT_NO_MEM path instead of throwing past it.
- Reentrancy guard restored via MemTrackerSuppress RAII on every path (hot path,
dump, reset) so an exception can't permanently disable tracking on a thread.
- Sampling reseed draws from [1, 2N-1] (mean exactly N); the old [1, 2N] biased
the Est* estimate low by ~0.5/N.
Design:
- Drop runtime enable/disable (now an explicit Non-requirement): the sample knob
is read at startup only; park the counter when off. Removes the DISABLED_RESEED
re-park and the on/off reconciliation logic.
- Simulation samples 1-in-10 (was 1-in-2).
Portability (Windows is not build-tested here; lean on existing abstractions):
- Aligned operator new uses platform::aligned_alloc/aligned_free (overflow-guarded)
instead of posix_memalign; guard <pthread.h> under __linux__; add a
force_noinline macro (GNU-only, empty elsewhere); portable volatile-sink
escape() in the test.
Tooling/tests/docs:
- mako A/B scripts: validate the scratch mount (realpath + tmpfs + denylist) before
rm -rf, locate mako_storage_bench.sh via __file__, and black-format.
- Add operatorNewHonorsNewHandler and samplingRate tests; drop enableAfterOff.
- Reconcile the design doc with the code; prune low-value comments.
Testing:
- /flow/MemoryTracker/* unit tests: 11 pass, 0 fail (bin/fdbserver -r unittests).
- Joshua 100k (correctness-8.0.0): 99,995 pass / 1 fail. The single failure is
NativeCdcAssignmentPublication -- a NativeCdc test from main, not this PR
(CommitProxy failed_to_progress -> QuietDatabase DataDistributionActive ->
NativeCdcEndToEnd workload start timed_out). It reproduces bit-identically
(seed 2939264355, unseed 8776) with the tracker built off (sim
sample_inverse=0, confirmed via the MemoryTrackerSummary SampleInverse trace
field), so it is a pre-existing rare CDC/QuietDatabase flake unrelated to this
change.
* memory-tracker: fix GCC IPA-clone breaking frame-attribution tests
Under GCC -O3, IPA constant-propagation cloning (-fipa-cp-clone) specializes the
MemoryTrackerTest sentinels (each called with a constant N) into `.constprop`
clones emitted at a different address than the function symbol. The executed
code -- and thus the captured return addresses -- live in the clone, so the
tests' frameInside(frame, &sentinel) window missed them: fastAlloc32Accounting
aborted (sitesWithSentinelFrames == 0) in GCC CI while clang passed.
Add `noclone` to force_noinline on GCC so each sentinel stays a single body at
the address &fn yields. clang doesn't support noclone (and doesn't clone this
way), so it keeps noinline only; other compilers stay empty.
Testing: /flow/MemoryTracker/* passes 11/11 under both a gcc-toolset-13 build
and a clang build.
* memory-tracker: cheaper disabled alloc hot path (per-thread off flag)
When sampling is off, memTrackerOnAlloc previously still did three TLS accesses
every allocation -- a gInMemTracker load, a gMemTrackerCounter load-decrement-
STORE, and a gForceSampleBytes load. Add a per-thread gMemTrackerOff flag,
checked first, that a thread sets once its slow path observes sampling is off;
the disabled alloc path then short-circuits on a single TLS load + branch (no
counter store, no gForceSampleBytes load).
The flag is per-thread, not global, on purpose: the counter still bootstraps
sampling per thread (first alloc reaches the slow path and reads the knob), so a
single global gate set by an early main-thread allocation before FLOW_KNOBS is
ready would wrongly disable worker threads that bootstrap later. Free keeps the
global g_memTrackerEnabled gate (a free-only thread must see global state to
debit). memTrackerResetForTest clears the flag. Removes the now-dead INT_MAX
counter parking.
The saving is real but small (~2 loads + 1 store per alloc, well under the
mako A/B's run-to-run noise), so the redwood off-state A/B shows no resolvable
change; the win is on principle / at high allocation rates.
Testing: /flow/MemoryTracker/* passes 11/11 (bin/fdbserver -r unittests).
* memory-tracker: add FDB_MEMORY_TRACKER compile-time gate + per-path microbench
Add a compile-time switch, FDB_MEMORY_TRACKER (CMake option, default ON), that
removes the feature entirely when set to 0: the header hooks become no-op inlines,
flow/MemoryTracker.cpp and the MemoryTrackerTest TEST_CASEs are #if'd out (the
forceLink stub stays), and fdbserver/GlobalNewDelete.cpp defines no global
operator new/delete override (libc++'s allocator is used, which still honors the
installed new_handler). This gives operators an escape hatch to zero
always-compiled footprint, and lets the microbench measure present-but-disabled
vs absent cost per allocation path.
Extend fdbserver/bench/BenchMemoryTracker.cpp with plain per-path alloc/free loops
(operator new[] at several sizes, FastAllocator<64/96/256>, Arena medium/huge) so
the same binary built =1 (tracker present, sampling off by default) vs =0 (absent)
isolates each path's unweighted off-state overhead.
Measured (EPYC 9R14 @ 3.7 GHz, ns/op, =1 minus =0):
operator new[] small ~+1.1 ns (~11%); huge ~+2.5 ns
FastAllocator<N> ~0 (within the ~0.6 ns cross-build noise floor)
Arena block ~+3.3 ns (medium) / +4.8 ns (huge)
The per-op-costly paths (operator new, Arena) are the low-volume ones; the
high-volume path (FastAllocator, ~82% of redwood allocs) is ~free, so the weighted
off-state overhead is ~0.1-0.2% of a core -- under the R0 target, though R0 is now
framed as an unproven target with this gate as the escape hatch.
Testing: /flow/MemoryTracker/* passes 11/11 (default build); FDB_MEMORY_TRACKER=OFF
builds clean under -Werror.
* clang-tidy fix
* comment about why fdbserver/bench exists
* address Codex round 5 review comments (MSVC build, startup sequencing, memory allocation failures on tracker-internal bookkeeping, cross-compile, contrib script cleanup)
* add another disclaimer about MSVC/Windows being best effort
* memory-tracker: fail-open ordering + deflake huge-arena unit test
memTrackerSampleAlloc now performs both table insertions (aggregation-map and
live-map nodes) before mutating any per-site or global counter, so if the
tracker's own map growth throws std::bad_alloc the exception unwinds with all
totals in lockstep and the fail-open catch simply drops the sample. This
addresses an adversarial-review finding. Note such a failure does not occur in
practice: FDB "OOM" is an RSS threshold enforced by fdbmonitor (typically
~12-16 GB against an ~8 GB target), not a malloc/operator-new failure. This
feature is for finding leaks and untuned allocations that drive RSS growth,
well short of any allocation failure -- documented in flow/MemoryTracker.cpp and
design/memory-tracker.md so reviewers don't over-index on the bad_alloc path.
arenaHugeAccounting now identifies the huge blocks by their ~100 KB size
signature instead of the frame-pointer sentinel. The huge-Arena path is the
deepest tracked call chain; under a real (non-simulation) network the
best-effort frame-pointer walker is per-allocation nondeterministic there and
cannot reliably attribute the blocks to the test's frame, so the sentinel-frame
assertion flaked (~1-4% of runs). The recorded block size is correct regardless
of which frames were captured, no incidental or foreign-thread allocation comes
near 100 KB, and double-tracking still shows up as 2N. The shallower
fastAlloc32/arenaSmall/arenaMedium/operatorNew accounting tests keep exact
sentinel-frame attribution (reliable on their shorter paths) as the regression
guard.
Testing:
- /flow/MemoryTracker/* via `fdbserver -r unittests`: 500 runs across two seed
spaces (including the sequence that previously flaked), 14/14 test cases pass
every run, 0 failures.
- Builds clean with FDB_MEMORY_TRACKER on and off under -Werror; clang-format
and clang-tidy clean on the changed files.
- Joshua 100k on the parent commit: ensemble
20260728-160205-gglass-0b30fa74f2605807, ended=100000 pass=100000 fail=0.
These changes are a pure counter-update reorder (no simulation-reachable
behavior change -- bad_alloc is not injected in sim), a unit-test-only change,
and documentation, so that result remains representative.
* memory-tracker: note why the fdbserver-local bench binary exists
Add a short comment to BenchMain.cpp explaining that most microbenchmarks
belong in flow/bench and this fdbserver-local benchmark binary exists only for
benchmarks that must link fdbserver-only code, pointing at BenchMemoryTracker.cpp
for the detailed rationale rather than duplicating it.
aa705c8c02
