* 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.
* Add configurable DNS removal delay in simulator
Co-authored-by: Renish Patel <renishpatel2482001@gmail.com>
* Fix DNS removal delay to run on surviving process
Co-authored-by: Renish Patel <renishpatel2482001@gmail.com>
---------
Co-authored-by: Renish Patel <renishpatel2482001@gmail.com>
Prior efforts (PR#12435, PR#12470, PR#12486, PR#12583, PR#12667, PR#12903) removed some experimental features. This PR cleans up some leftover pieces.
I suppose this is basically a matter of historical interest at this point but those PRs were done purely by hand (ok, with like grep and a text editor) i.e. without any AI coding assistance. So it is not surprising that there is a few percent of leftover bits here and there.
Fixes:
Remove monitorBlobWorkers() and blobRestoreCommandActor() declarations that had no implementations (linker bombs if called)
Remove fast_restore from setclass help text (previously directed operators into an ASSERT(false) crash)
Dead knob removal (13 knobs):
12 blob worker ratekeeper knobs (BW_THROTTLING_ENABLED, TARGET_BW_LAG, etc.) and BLOB_WORKER_PAGE_CACHE
Remove bwLagTarget field from RatekeeperLimits and associated plumbing in Ratekeeper
Dead code removal:
4 never-incremented storage server counters (feedBytesFetched, changeFeedMutations, changeFeedMutationsDurable, changeFeedDiskReads)
Assigned-but-never-read nonExpanded variable in StorageServer::addMutation
ClusterNameRef/ClusterName typedefs, RestoreLoader/Applier/Master forward declarations
Dead schema strings (blob_worker_lag, blob_worker_missing, unreachable_blobManager_worker, metacluster_metrics_missing)
Dead cacheKeys*/cacheChange* function implementations in SystemData.cpp
Dead tenant group code in FuzzApiCorrectness.cpp
Dead fdbcli constants (msgClusterTypeKey, msgDataClustersKey) and blobrange history filter
Dead file removal:
contrib/mockkms/ -- unused Go mock KMS server
tests/fast/EncryptionUnitTests.toml -- tests a path (/blobCipher) that no longer exists
* - Compare storage replicas on reads (in "loadBalance()")
* - Do consistency check on reads in loadbalance
* - Do replica consistency check in the case where loadBalance issues
requests to multiple storage servers
* - Address a state variable related bug
* - Code formatting
* - API simplification
* - Simplify code
* - Code formatting
* - Address a review comment
* EaR: reduce metrics logging
BlobCipherMetrics used to break down by usage types (whehter it is for tlog, redwood, backup, etc), and these counters will be printed to trace log even when encryption is not enabled, or the specific usage is not happening on a node (e.g. a node with only stateless roles will also print blob cipher counters for redwood). We are reducing the BlobCipherMetrics loggings by:
1. Default to not breakdown the metrics by usage type, and the behavior is controlled by the knob `ENCRYPT_KEY_CACHE_ENABLE_DETAIL_LOGGING`
2. When the detail breakdown is enabled, the counters are lazily initialize
3. Even if the counters are initialized, they will not be logged if the count is 0 (so like if a node was recruited as tlog but then drops the tlog role later on, the tlog counter inside BlobCipherMetrics will not be logged anymore).
* buggify BlobCipherMetrics detail logging knob
* format
* EaR: REST KMS fixes - encryption integration testing
Description
Major changes:
1. Multiple fixes observed while performing integration end-to-end
testing for Encryption at-rest feature.
2. Improve REST module logging. Introduced FLOW_KNOBS->REST_LOG_LEVEL
to have more granular control of feature logging disconnected from
the cluster log level.
Testing
Integration testbed:
1. Run fdbserver standalone
2. Run external KMS http-server to serve encryption key fetch requests
* Define API for unsuppressable TraceEvent types
Add trace checking tests for authz trace events
* Revert temporary configurations used for debugging
* Simplify/Modernize flow audit logging API
- Do event type whitelist checks at compile time
- Use ""_audit literal API instead of a tag struct
- Replace int with a lightweight struct for tracking/modifying TraceEvent enablement
* Revert installing signal handler for SIGTERM and refactor test script
Move trace checker to local_cluster.py
* Lengthen public key refresh interval and add more audited events
* Try and make MSVC and Mac build happy
* consteval > constexpr
'inline consteval' still causes link errors in Mac builds
Previously with EaR we always enable authentication (e.g. we encrypt Redwood pages). The authentication is a form of checksum, so dedicated page checksum was not needed. This PR adds back xxhash page checksum when authentication is disabled. Also change the knob to default disable authentication.
gxglass