cache: shared-memory-backed Dir for fast restart#13328
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in “fast restart” cache directory implementation by hosting each stripe’s in-memory directory (raw_dir) in POSIX shared memory, allowing subsequent traffic_server starts to attach the prior directory quickly instead of rebuilding from disk. It also adds operator tooling (traffic_ctl cache shm status|clear), configuration records, extensive AuTest coverage, unit tests for trust gates, and design/admin documentation.
Changes:
- Add shared-memory directory infrastructure (
CacheShm*) plus integration into cache startup, stripe directory allocation, and shutdown/clean-marking paths. - Add
traffic_ctl cache shm status|clearcommands and newproxy.config.cache.shm.*records. - Add unit + AuTest suites and documentation for the shm fast-restart feature.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/gold_tests/logging/ts_process_handler.py | Hardens psutil cmdline handling for macOS/permission-restricted processes. |
| tests/gold_tests/cache/shm_poke.py | Adds Linux-only helper to tamper with /dev/shm segments for trust-gate testing. |
| tests/gold_tests/cache/replay/cache-shm-fast-restart.replay.yaml | Proxy Verifier replay traffic used to validate hit-after-restart behavior. |
| tests/gold_tests/cache/gold/cache_shm_state_after_shutdown.gold | Gold output for traffic_ctl cache shm status validation. |
| tests/gold_tests/cache/cache_shm_unclean_shutdown.test.py | AuTest for rejecting dirty segments after SIGKILL and rebuilding from disk. |
| tests/gold_tests/cache/cache_shm_storage_mismatch.test.py | AuTest for partial attach behavior when storage layout changes. |
| tests/gold_tests/cache/cache_shm_schema_mismatch.test.py | Linux-only AuTest that pokes schema_version and verifies drop+rebuild. |
| tests/gold_tests/cache/cache_shm_purge_on_disable.test.py | AuTest for purge-on-disabled-start behavior and traffic_ctl exit codes. |
| tests/gold_tests/cache/cache_shm_fast_restart.test.py | End-to-end AuTest for clean shutdown -> shm attach -> cache HIT without origin contact. |
| tests/gold_tests/cache/cache_shm_concurrent_attach.test.py | AuTest for concurrent-attach guard (flock vs owner_pid liveness backstop). |
| tests/gold_tests/cache/cache_shm_bad_disk_dropped.test.py | AuTest for partial attach and orphan reclaim when a disk is removed from storage. |
| src/traffic_ctl/traffic_ctl.cc | Adds `traffic_ctl cache shm status |
| src/traffic_ctl/CMakeLists.txt | Builds the new traffic_ctl command source and adds cache include path for shared headers. |
| src/traffic_ctl/CacheShmCommand.h | Declares the traffic_ctl cache shm command handler. |
| src/traffic_ctl/CacheShmCommand.cc | Implements shm status/clear via direct shm_open/mmap, shared purge primitive, and exit codes. |
| src/records/RecordsConfig.cc | Registers proxy.config.cache.shm.* configuration records. |
| src/iocore/cache/unit_tests/test_CacheShm.cc | Adds unit tests for ABI hash, storage signature, layout round-trip, prefix normalization, and liveness checks. |
| src/iocore/cache/StripeSM.cc | Adds fast-attach path that can skip disk dir read + recovery when shm directory is trusted; adjusts shutdown behavior. |
| src/iocore/cache/Stripe.h | Adds _shm_directory_is_valid() and strengthens flush API via [[nodiscard]]. |
| src/iocore/cache/Stripe.cc | Uses shm-backed directory allocation; adds shm bounds validation; detaches shm mappings safely in destructor. |
| src/iocore/cache/CMakeLists.txt | Builds new CacheShm.cc and adds test_CacheShm to the cache unit tests. |
| src/iocore/cache/CacheShmPurge.h | Introduces shared header-only purge/enumerate/unlink primitive for server startup and traffic_ctl. |
| src/iocore/cache/CacheShmLayout.h | Defines the shared control-segment layout and prefix normalization utilities. |
| src/iocore/cache/CacheShm.h | Declares the CacheShm facade for lifecycle + stripe attach/create + trust gates. |
| src/iocore/cache/CacheShm.cc | Implements shm control segment lifecycle, trust gates, stripe attach/create, orphan reclaim, and clean-shutdown marking. |
| src/iocore/cache/CacheProcessor.cc | Calls CacheShm initialization before stripe construction and finalization after cache init. |
| src/iocore/cache/CacheDir.cc | Marks shm control segment clean only after shutdown sync has quiesced writers. |
| src/iocore/cache/AggregateWriteBuffer.h | Marks flush() as [[nodiscard]] to encourage handling short-write failures. |
| src/iocore/cache/AggregateWriteBuffer.cc | Converts flush failure from an assertion to a false return for graceful handling. |
| doc/developer-guide/cache-architecture/shm-fast-restart.en.rst | Adds detailed design doc: layout, gates, attach modes, shutdown semantics, tooling, and platform notes. |
| doc/developer-guide/cache-architecture/index.en.rst | Hooks shm fast-restart document into the cache architecture index. |
| doc/admin-guide/files/records.yaml.en.rst | Documents new proxy.config.cache.shm.* records and operator-facing behavior/tooling. |
Cold-start cache initialization rebuilds each stripe's in-memory directory from disk on every restart -- multi-minute on large caches. Host the directory in POSIX shared memory so the next process start attaches the existing segment in milliseconds instead of rebuilding it. Recovery stays binary and fail-safe: when the segment cannot be trusted (crash, reboot, ABI/schema or storage mismatch, failed validation) the start drops it and rebuilds via the existing disk path, and reads still validate Doc magic + key so a stale entry is a miss, never corruption. Opt-in behind proxy.config.cache.shm.enabled (default 0), where it is a functional no-op.
8358d37 to
e0ee0af
Compare
POSIX shm names permit only the leading '/', so a misconfigured name_prefix like "foo/bar" would build a name shm_open rejects with EINVAL. Strip embedded '/' during normalization instead of preserving it.
On glibc < 2.34 (e.g. CentOS 7) shm_open/shm_unlink live in librt, so traffic_ctl and inkcache fail to link. Add an optional rt::rt target that is a no-op where the library is folded into libc (modern glibc, macOS).
There was a problem hiding this comment.
Approving. This is well built and safe to land. It is opt-in and off by default, the shared-memory attach path validates the mapping size before use and bounds the stripe count, and every failing system call is logged with its name and error string and then propagated back to the caller.
One correction to my earlier note. I wrote that a fast-attach bug returning a wrong offset or length would still pass CI. That is not accurate, and I withdraw it. The fast_restart test wires the origin to return a 502 on the hit transaction, so if the shared-memory directory failed to resolve the cached object, ATS would fall through to the origin and the 502 would fail the test. The real gap is narrower than I first stated.
The items below are all non-blocking for an off-by-default feature, but worth closing out before this is relied on in production.
-
The testing table in
doc/developer-guide/cache-architecture/shm-fast-restart.en.rstlists acache_shm_data_integrityautest that is not in this PR. Please either add the test or drop the row. -
The objects the tests serve are small and single-fragment. A multi-fragment or large-object case after a fast attach would cover the one path the 502 trick does not fully exercise.
-
The rejection branches in
_shm_directory_is_valid()and the invalidate-on-shutdown paths have no direct unit coverage, as the caveats note. They are close to pure header math, so each branch is a cheap unit test.
I also agree with the open Copilot threads on this PR.
-
In
CacheShm.cc:229,unlink_all_known_segments()unlinks stripe names read from the control segment. On the path that drops an untrusted or corrupt control segment those names are untrusted, so a corrupt segment could lead toshm_unlinkon unrelated objects. Since the names are always built fromname_prefixviabuild_stripe_shm_name(), gating the unlink loop on a prefix match, and only running it when the control magic is valid, closes this with no effect on the normal path. -
The three
storage.configreferences in the design doc at lines 132, 340, and 430 should bestorage.yamlto match the tests and the rest of this PR.
Thanks for the clean, well-documented change. The approval stands.
A corrupt control segment (invalid magic) left the stripe-name bytes untrusted, yet the drop path unlinked them, so a bad segment could shm_unlink unrelated POSIX objects. Only walk the stripe table when the header magic is intact and restrict unlinks to our own name prefix, matching the guard already in purge_segments(). Also correct storage.config -> storage.yaml in the design doc and drop the testing-table row for the not-yet-added cache_shm_data_integrity autest.
|
Thank you for taking a look. I addressed 1. 4. and 5. by 0cc3b52. |
| if (addr == MAP_FAILED) { | ||
| int e = errno; | ||
| Warning("mmap(%s, %zu) failed: %s", name.c_str(), size, strerror(e)); | ||
| if (out_errno != nullptr) { | ||
| *out_errno = e; | ||
| } | ||
| return nullptr; | ||
| } |
| ++reclaimed; | ||
| } | ||
| if (reclaimed > 0) { | ||
| Note("cache shm: reclaimed %u orphaned stripe segment(s) after storage change", reclaimed); |
Address review feedback on the shm-backed Dir. Create shm segments with O_EXCL so a create never adopts a pre-existing object; otherwise a segment planted by another user could hand ATS attacker-controlled permissions and contents. Both create paths already unlink stale names first, so this only rejects races and pre-planted files. Enabling shm previously preempted the MAP_HUGETLB dir allocation silently, dropping a box that relied on the global hugepage allocator to base tmpfs pages. shm segments cannot use MAP_HUGETLB, so inherit the allocator's intent by advising MADV_HUGEPAGE (THP) when shm.use_hugepages is left at default, honor an explicit opt-out, and log the substitution once.
91899f8 to
161f39c
Compare
A storage.yaml change does not discard the shm segments; storage_ signature is informational, not a trust gate. Each stripe attaches by its own identity, so unchanged stripes fast-attach, added or resized stripes rebuild from disk, and vanished stripes are reclaimed. Drop the misleading discard bullet and describe the partial attach instead.
Huge pages back the shm directory to cut page-table teardown cost at process exit -- the restart-speed motivation for the feature -- not merely to reduce TLB pressure. Align the admin guide with the design doc and code.
At graceful shutdown a shm-backed stripe with an aggregate write still in flight would have write_pos advanced twice -- once by the shutdown flush, again by the deferred aggWriteDone -- so the fast-attach path would trust an imprecise cursor verbatim. Invalidate the shm copy and fall through to a fresh on-disk directory write, so the next start recovers via recover_data() with a short scan instead of a stale periodic-sync copy. Also reword the mark_clean_shutdown comment, which overstated that all writers are stopped; the real guarantee is the read-path magic+key check that turns any late write into a miss, never served corruption.
Fold the PR 13328 review fixes into the feature: - unlink the shm segment on mmap failure in the create path, so a failed create does not leak it and wedge the next O_EXCL create on EEXIST (silently disabling shm). - harden purge_segments(): filter stripe unlinks to the configured prefix so a corrupt-but-magic-valid table cannot touch unrelated shm, and report an fstat failure instead of unlinking the control object. - retry the control flock on EINTR so a signal does not drop the guard to the owner-pid backstop; reword the orphan-reclaim log to "after attach" since it also fires on disk-open failure. - drop the redundant cmdline guard and stale comment in the logging test helper, and tighten the longest shm comment blocks. No functional change to the fast-restart path itself.
moonchen
left a comment
There was a problem hiding this comment.
Went through the whole PR again -- the earlier threads are all addressed (resolved them). Three asks below: the wrong-size control-segment wedge, tests for the _shm_directory_is_valid rejection branches, and a structural check on the fast-attach path. Plus two optional suggestions.
| }; | ||
| std::size_t expected_max = INK_ALIGN(size, ats_pagesize()); | ||
| if (fstat(fd, &sb) < 0 || sb.st_size < 0 || static_cast<std::size_t>(sb.st_size) < size || | ||
| static_cast<std::size_t>(sb.st_size) > expected_max) { |
There was a problem hiding this comment.
This wedges shm off permanently after an upgrade that changes sizeof(CacheShmControl) (a MAX_STRIPES bump, a longer shm_name, a new StripeEntry field): the open fails here, initialize() never reaches the abi_hash drop path (that needs a successful map), and the O_EXCL create then fails with EEXIST on every restart -- until someone runs traffic_ctl cache shm clear. The CacheShmLayout.h comment ("Bumping it changes the ABI hash, so old segments are dropped automatically") only holds for same-size changes.
Could initialize() treat a wrong-size (or any non-ENOENT) open failure as a stale segment -- take the flock/owner-pid guard, unlink, and fall through to the fresh create? (Alternatively: a fixed-size preamble that is mapped and checked before the full-size map, so the version fields stay readable across layout changes.)
| // re-validate individual Dir entries -- the read path already checks Doc magic + key | ||
| // before serving, so a stale entry resolves to a miss, never served corruption. | ||
| bool | ||
| Stripe::_shm_directory_is_valid() const |
There was a problem hiding this comment.
Since these rejection code branches are critical for preventing cache corruption, I think they should be tested. shm_poke.py can poke the stripe segment header in /dev/shm after a clean shutdown (e.g. set write_pos past the stripe, or freelist[0] out of range); a restart should then log "shm directory invalid ... falling back to disk read" and still serve a HIT after disk recovery.
| this->directory.header->last_write_pos < data_lo || this->directory.header->last_write_pos > data_hi || | ||
| this->directory.header->agg_pos < data_lo || this->directory.header->agg_pos > data_hi) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
One optional suggestion: a clean shutdown also guarantees the write cursor is quiesced (flush_aggregate_write_buffer and aggWriteDone both maintain it, and an in-flight AIO invalidates the segment), so the validator can require it -- the runtime asserts that would otherwise catch this are debug-only.
| } | |
| } | |
| // A clean shutdown leaves the write cursor quiesced; a mismatch means this is | |
| // not a clean-shutdown artifact. | |
| if (this->directory.header->agg_pos != this->directory.header->write_pos || | |
| this->directory.header->last_write_pos > this->directory.header->write_pos) { | |
| return false; | |
| } |
|
|
||
| int fd = shm_open(control_name.c_str(), O_RDONLY, 0); | ||
| if (fd < 0) { | ||
| std::cerr << "cache shm: control segment '" << control_name << "' not found: " << std::strerror(errno) << '\n'; |
There was a problem hiding this comment.
One optional suggestion: an operator with a custom proxy.config.cache.shm.name_prefix gets "not found" against the default ats prefix and may conclude shm was never enabled. Worth a hint here (and in clear()'s NotPresent message):
| std::cerr << "cache shm: control segment '" << control_name << "' not found: " << std::strerror(errno) << '\n'; | |
| std::cerr << "cache shm: control segment '" << control_name << "' not found: " << std::strerror(errno) | |
| << " (if proxy.config.cache.shm.name_prefix is set, pass --prefix <word>)\n"; |
| // header/footer, jump straight to dir_init_done() in the normal post-recovery | ||
| // state. Validation failure falls through to disk read + recover_data(). | ||
| if (CacheShm::mode() == CacheShm::Mode::AttachExisting && CacheShm::is_shm_pointer(this->directory.raw_dir)) { | ||
| if (this->directory.header->magic == STRIPE_MAGIC && this->directory.footer->magic == STRIPE_MAGIC && |
There was a problem hiding this comment.
mark_clean_shutdown() runs while event threads are still live, and the main thread exits without joining them (the wait loop in traffic_server.cc wakes on the same flag and calls exit) -- so a dir mutation torn at process exit can leave a structurally inconsistent directory behind a clean_shutdown=1 flag. Since there's no cheap way to quiesce writers first, could the fast path also run Directory::check() (the CHECK_DIR walk: bucket bounds, chain consistency, loop detection) and fall back to the disk read when it fails? It's an in-memory walk -- small next to the rebuild it replaces -- and it turns a torn dir into a rebuild instead of an attach.
Motivation
Cold-start cache initialization rebuilds each stripe's in-memory directory from disk on every
restart — multi-minute on large caches.
Approach
Host the directory (
raw_dir) in POSIX shared memory so the next process start attaches theexisting segment in milliseconds instead of rebuilding it. Recovery is binary and fail-safe:
anything untrustworthy (crash, reboot, ABI/schema mismatch, failed validation, bad disk) falls
back to the existing disk-rebuild path, and reads still validate
Docmagic + key so a staleentry is a miss, never served corruption.
New configs & traffic_ctl commands
Opt-in behind
proxy.config.cache.shm.enabled(default0, a functional no-op). Also addsproxy.config.cache.shm.{name_prefix,use_hugepages,purge_stale_on_start}and atraffic_ctl cache shm status|clearcommand.Details
Design, recovery model, configuration, and platform notes are in
doc/developer-guide/cache-architecture/shm-fast-restart.en.rst(added in this PR) and theproxy.config.cache.shm.*entries inrecords.yaml.Testing
test_CacheShm(ABI hash, storage signature, control round-trip, name length, prefixnormalization, process liveness).
cache_shm_*suites — fast restart, unclean shutdown, schema/storage mismatch,bad-disk drop + orphan reclaim, concurrent-attach refusal, purge-on-disable.
Caveats
_shm_directory_is_valid()rejection branches or thebad-disk / flush-failure
invalidate_stripe_directory()paths (planned follow-up).flockguard is authoritative on Linux but ano-op on macOS POSIX shm, where it falls back to an owner-pid liveness check.