From d98a6c2552b599d619dca26ca241b85be41a0b92 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Wed, 10 Jun 2026 21:38:00 +0000 Subject: [PATCH 01/13] Add lz4 support to CLFUS --- CMakeLists.txt | 5 +++ ci/docker/deb/Dockerfile | 2 +- cmake/FindLZ4.cmake | 43 +++++++++++++++++++ doc/admin-guide/files/records.yaml.en.rst | 3 +- doc/admin-guide/storage/index.en.rst | 1 + .../cache-architecture/ram-cache.en.rst | 21 +++++---- include/iocore/cache/Cache.h | 12 +++++- include/tscore/ink_config.h.cmake.in | 1 + src/iocore/cache/CMakeLists.txt | 4 ++ src/iocore/cache/CacheProcessor.cc | 5 +++ src/iocore/cache/RamCacheCLFUS.cc | 34 ++++++++++++++- src/traffic_layout/CMakeLists.txt | 4 ++ src/traffic_layout/info.cc | 15 +++++++ 13 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 cmake/FindLZ4.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 897cf30df74..5ae64050c9d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -516,6 +516,11 @@ else() set(HAVE_ZSTD_H FALSE) endif() +find_package(LZ4) +if(LZ4_FOUND) + set(HAVE_LZ4_H TRUE) +endif() + # ncurses is used in traffic_top find_package(Curses) set(HAVE_CURSES_H ${CURSES_HAVE_CURSES_H}) diff --git a/ci/docker/deb/Dockerfile b/ci/docker/deb/Dockerfile index 85815163e2f..cd7d4bb89bf 100644 --- a/ci/docker/deb/Dockerfile +++ b/ci/docker/deb/Dockerfile @@ -56,7 +56,7 @@ RUN apt-get update; apt-get -y dist-upgrade; \ libhwloc-dev libunwind8 libunwind-dev zlib1g-dev \ tcl-dev tcl8.6-dev libjemalloc-dev libluajit-5.1-dev liblzma-dev \ libhiredis-dev libbrotli-dev libncurses-dev libgeoip-dev libmagick++-dev \ - libzstd-dev; \ + libzstd-dev liblz4-dev; \ # Optional: This is for the OpenSSH server, and Jenkins account + access (comment out if not needed) apt-get -y install openssh-server openjdk-8-jre && mkdir /run/sshd; \ groupadd -g 665 jenkins && \ diff --git a/cmake/FindLZ4.cmake b/cmake/FindLZ4.cmake new file mode 100644 index 00000000000..c36a6573c32 --- /dev/null +++ b/cmake/FindLZ4.cmake @@ -0,0 +1,43 @@ +find_path( + LZ4_INCLUDE_DIR + NAMES lz4.h + DOC "lz4 include directory" +) +mark_as_advanced(LZ4_INCLUDE_DIR) +find_library( + LZ4_LIBRARY + NAMES lz4 liblz4 + DOC "lz4 library" +) +mark_as_advanced(LZ4_LIBRARY) + +if(LZ4_INCLUDE_DIR) + file(STRINGS "${LZ4_INCLUDE_DIR}/lz4.h" _lz4_version_lines REGEX "#define[ \t]+LZ4_VERSION_(MAJOR|MINOR|RELEASE)") + string(REGEX REPLACE ".*LZ4_VERSION_MAJOR *\([0-9]*\).*" "\\1" _lz4_version_major "${_lz4_version_lines}") + string(REGEX REPLACE ".*LZ4_VERSION_MINOR *\([0-9]*\).*" "\\1" _lz4_version_minor "${_lz4_version_lines}") + string(REGEX REPLACE ".*LZ4_VERSION_RELEASE *\([0-9]*\).*" "\\1" _lz4_version_release "${_lz4_version_lines}") + set(LZ4_VERSION "${_lz4_version_major}.${_lz4_version_minor}.${_lz4_version_release}") + unset(_lz4_version_major) + unset(_lz4_version_minor) + unset(_lz4_version_release) + unset(_lz4_version_lines) +endif() + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args( + LZ4 + REQUIRED_VARS LZ4_LIBRARY LZ4_INCLUDE_DIR + VERSION_VAR LZ4_VERSION +) + +if(LZ4_FOUND) + set(LZ4_INCLUDE_DIRS "${LZ4_INCLUDE_DIR}") + set(LZ4_LIBRARIES "${LZ4_LIBRARY}") + + if(NOT TARGET LZ4::LZ4) + add_library(LZ4::LZ4 UNKNOWN IMPORTED) + set_target_properties( + LZ4::LZ4 PROPERTIES IMPORTED_LOCATION "${LZ4_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}" + ) + endif() +endif() diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 5d75b68bafc..cd154186129 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -3178,9 +3178,10 @@ RAM Cache Value Description ======== =================================================================== ``0`` No compression - ``1`` Fastlz (extremely fast, relatively low compression) + ``1`` Fastlz (extremely fast, relatively low compression) - prefer lz4 ``2`` Libz (moderate speed, reasonable compression) ``3`` Liblzma (very slow, high compression) + ``4`` lz4 (extremely fast, relatively low compression) ======== =================================================================== Compression runs on task threads. To use more cores for RAM cache diff --git a/doc/admin-guide/storage/index.en.rst b/doc/admin-guide/storage/index.en.rst index af6d23de065..88f9816c754 100644 --- a/doc/admin-guide/storage/index.en.rst +++ b/doc/admin-guide/storage/index.en.rst @@ -109,6 +109,7 @@ Value Meaning 1 *fastlz* compression 2 *libz* compression 3 *liblzma* compression +4 *lz4* compression ======= ============================= .. _changing-the-size-of-the-ram-cache: diff --git a/doc/developer-guide/cache-architecture/ram-cache.en.rst b/doc/developer-guide/cache-architecture/ram-cache.en.rst index bb6851e2e2d..690888df5b3 100644 --- a/doc/developer-guide/cache-architecture/ram-cache.en.rst +++ b/doc/developer-guide/cache-architecture/ram-cache.en.rst @@ -37,7 +37,7 @@ following features: * Is Scan Resistant and extracts robust hit rates even when the working set does not fit in the RAM Cache. -* Supports compression at 3 levels: fastlz, gzip (libz), and xz (liblzma). +* Supports compression at 4 levels: fastlz, gzip (libz), xz (liblzma) and lz4. Compression can be moved to another thread. * Has very low CPU overhead, only slightly more than a basic LRU. Rather than @@ -72,7 +72,7 @@ len Length of the object, which differs from *size* because of compression and padding). compressed_len Compressed length of the object. compressed Compression type, or ``none`` if no compression. Possible types - are: *fastlz*, *libz*, and *liblzma*. + are: *fastlz*, *libz*, *liblzma*, and *lz4*. uncompressible Flag indicating that content cannot be compressed (true), or that it mat be compressed (false). copy Whether or not this object should be copied in and copied out @@ -147,17 +147,20 @@ since we need to make a copy anyway. Those not tagged ``copy`` are inserted uncompressed in the hope that they can be reused in uncompressed form. This is a compile time option and may be something we want to change. -There are 3 algorithms and levels of compression (speed on an Intel i7 920 -series processor using one thread): +There are 4 algorithms and levels of compression (speed on an Intel Xeon Gold +6338 processor using lzbench and the silesia XML benchmark): ======= ================ ================== ==================================== Method Compression Rate Decompression Rate Notes ======= ================ ================== ==================================== -fastlz 173 MB/sec 442 MB/sec Basically free since disk or network - will limit first; ~53% final size. -libz 55 MB/sec 234 MB/sec Almost free, particularly - decompression; ~37% final size. -liblzma 3 MB/sec 50 MB/sec Expensive; ~27% final size. +fastlz 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. + Basically free since disk or network + will limit first; ~26% final size. +libz 54 MB/sec 536 MB/sec Almost free, particularly + decompression; ~13% final size. +liblzma 5 MB/sec 291 MB/sec Expensive; ~8% final size. +lz4 727 MB/sec 3458 MB/sec Basically free since disk or network + will limit first; 23% final size ======= ================ ================== ==================================== These are ballpark numbers, and your millage will vary enormously. JPEG, for diff --git a/include/iocore/cache/Cache.h b/include/iocore/cache/Cache.h index 9a0fe5d430b..a4529cb5ac6 100644 --- a/include/iocore/cache/Cache.h +++ b/include/iocore/cache/Cache.h @@ -44,8 +44,16 @@ static constexpr ts::ModuleVersion CACHE_MODULE_VERSION(1, 0); #define CACHE_COMPRESSION_FASTLZ 1 #define CACHE_COMPRESSION_LIBZ 2 #define CACHE_COMPRESSION_LIBLZMA 3 - -enum { RAM_HIT_COMPRESS_NONE = 1, RAM_HIT_COMPRESS_FASTLZ, RAM_HIT_COMPRESS_LIBZ, RAM_HIT_COMPRESS_LIBLZMA, RAM_HIT_LAST_ENTRY }; +#define CACHE_COMPRESSION_LZ4 4 + +enum { + RAM_HIT_COMPRESS_NONE = 1, + RAM_HIT_COMPRESS_FASTLZ, + RAM_HIT_COMPRESS_LIBZ, + RAM_HIT_COMPRESS_LIBLZMA, + RAM_HIT_COMPRESS_LZ4, + RAM_HIT_LAST_ENTRY +}; struct CacheVC; class CacheEvacuateDocVC; diff --git a/include/tscore/ink_config.h.cmake.in b/include/tscore/ink_config.h.cmake.in index 40b52686c7c..681c99581cc 100644 --- a/include/tscore/ink_config.h.cmake.in +++ b/include/tscore/ink_config.h.cmake.in @@ -47,6 +47,7 @@ #cmakedefine HAVE_POSIX_FALLOCATE 1 #cmakedefine HAVE_POSIX_MADVISE 1 #cmakedefine HAVE_ZSTD_H 1 +#cmakedefine HAVE_LZ4_H 1 #cmakedefine HAVE_PTHREAD_GETNAME_NP 1 #cmakedefine HAVE_PTHREAD_GET_NAME_NP 1 diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt index c9ba5bd2fde..14ad043e5de 100644 --- a/src/iocore/cache/CMakeLists.txt +++ b/src/iocore/cache/CMakeLists.txt @@ -58,6 +58,10 @@ if(HAVE_LZMA_H) target_link_libraries(inkcache PRIVATE LibLZMA::LibLZMA) endif() +if(HAVE_LZ4_H) + target_link_libraries(inkcache PRIVATE LZ4::LZ4) +endif() + if(BUILD_TESTING) # Unit Tests with unit_tests/main.cc macro(add_cache_test name) diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index c178aeb959d..d21f3c27d64 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -1677,6 +1677,11 @@ CacheProcessor::cacheInitialized() case CACHE_COMPRESSION_LIBLZMA: #ifndef HAVE_LZMA_H Fatal("lzma not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_LZ4: +#ifndef HAVE_LZ4_H + Fatal("lz4 not available for RAM cache compression"); #endif break; } diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index dcabd4c85c3..77f7bc349f0 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -37,6 +37,9 @@ #ifdef HAVE_LZMA_H #include #endif +#ifdef HAVE_LZ4_H +#include +#endif // #define CHECK_ACOUNTING 1 // very expensive double checking of all sizes @@ -123,6 +126,11 @@ RamCacheCLFUSCompressor::mainEvent(int /* event ATS_UNUSED */, Event *e) case CACHE_COMPRESSION_LIBLZMA: #ifndef HAVE_LZMA_H Warning("lzma not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_LZ4: +#ifndef HAVE_LZ4_H + Warning("lz4 not available for RAM cache compression"); #endif break; } @@ -257,6 +265,16 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey ram_hit_state = RAM_HIT_COMPRESS_LIBLZMA; break; } +#endif +#ifdef HAVE_LZ4_H + case CACHE_COMPRESSION_LZ4: { + int l = static_cast(e->len); + if (l != LZ4_decompress_safe(e->data->data(), b, e->compressed_len, l)) { + goto Lfailed; + } + ram_hit_state = RAM_HIT_COMPRESS_LZ4; + break; + } #endif } IOBufferData *data = new_xmalloc_IOBufferData(b, e->len); @@ -428,7 +446,12 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) break; #ifdef HAVE_LZMA_H case CACHE_COMPRESSION_LIBLZMA: - l = e->len; + l = static_cast(lzma_stream_buffer_bound(e->len)); + break; +#endif +#ifdef HAVE_LZ4_H + case CACHE_COMPRESSION_LZ4: + l = static_cast(LZ4_compressBound(e->len)); break; #endif } @@ -469,6 +492,15 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) l = static_cast(pos); break; } +#endif +#ifdef HAVE_LZ4_H + case CACHE_COMPRESSION_LZ4: { + int ll = l; + if ((l = LZ4_compress_default(edata->data(), b, elen, ll)) == 0) { + failed = true; + } + break; + } #endif } MUTEX_TAKE_LOCK(stripe->mutex, thread); diff --git a/src/traffic_layout/CMakeLists.txt b/src/traffic_layout/CMakeLists.txt index b86d4f2573a..ddc9c2674d8 100644 --- a/src/traffic_layout/CMakeLists.txt +++ b/src/traffic_layout/CMakeLists.txt @@ -35,6 +35,10 @@ if(HAVE_ZSTD_H) target_link_libraries(traffic_layout PRIVATE zstd::zstd) endif() +if(HAVE_LZ4_H) + target_link_libraries(traffic_layout PRIVATE LZ4::LZ4) +endif() + install(TARGETS traffic_layout) clang_tidy_check(traffic_layout) diff --git a/src/traffic_layout/info.cc b/src/traffic_layout/info.cc index 91b0677e042..6bc3ee67e06 100644 --- a/src/traffic_layout/info.cc +++ b/src/traffic_layout/info.cc @@ -54,6 +54,10 @@ #include #endif +#if HAVE_LZ4_H +#include +#endif + #if HAVE_SSL_CTX_ADD_CERT_COMPRESSION_ALG static constexpr int ts_has_cert_compression_callbacks = 1; #else @@ -133,6 +137,11 @@ produce_features(bool json) print_feature("TS_HAS_ZSTD", 1, json); #else print_feature("TS_HAS_ZSTD", 0, json); +#endif +#ifdef HAVE_LZ4_H + print_feature("TS_HAS_LZ4", 1, json); +#else + print_feature("TS_HAS_LZ4", 0, json); #endif print_feature("TS_HAS_CERT_COMPRESSION", ts_has_cert_compression, json); print_feature("TS_HAS_CERT_COMPRESSION_CALLBACKS", ts_has_cert_compression_callbacks, json); @@ -259,6 +268,12 @@ produce_versions(bool json) #else print_var("zstd", undef, json); #endif +#ifdef HAVE_LZ4_H + print_var("lz4", LBW().print("{}", LZ4_VERSION_STRING).view(), json); + print_var("lz4.run", LBW().print("{}", LZ4_versionString()).view(), json); +#else + print_var("lz4", undef, json); +#endif // This should always be last print_var("traffic-server", LBW().print(TS_VERSION_STRING).view(), json, true); From a61ac1ca371c761ec4f94c5e072cc820caed4fee Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Wed, 10 Jun 2026 22:18:41 +0000 Subject: [PATCH 02/13] Add unit tests --- src/iocore/cache/CMakeLists.txt | 1 + .../cache/unit_tests/test_RamCacheCLFUS.cc | 226 ++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt index 14ad043e5de..ecdce059bc7 100644 --- a/src/iocore/cache/CMakeLists.txt +++ b/src/iocore/cache/CMakeLists.txt @@ -97,6 +97,7 @@ if(BUILD_TESTING) add_cache_test(Update_Header unit_tests/test_Update_header.cc) add_cache_test(CacheStripe unit_tests/test_Stripe.cc) add_cache_test(CacheAggregateWriteBuffer unit_tests/test_AggregateWriteBuffer.cc) + add_cache_test(RamCacheCLFUS unit_tests/test_RamCacheCLFUS.cc) add_cache_test(RamCacheCompressEntries unit_tests/test_RamCacheCompressEntries.cc) # Only the shutdown test attaches a live segment; the rest need no shm syscall. add_cache_test(CacheShm unit_tests/test_CacheShm.cc) diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc new file mode 100644 index 00000000000..2e6f5dfb4b2 --- /dev/null +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -0,0 +1,226 @@ +/** @file + + Catch-based unit tests for RAM cache (CLFUS) compression roundtrips. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "main.h" + +#include "../RamCacheCLFUS.h" +#include "../P_CacheInternal.h" + +#include "iocore/cache/Cache.h" +#include "tscore/ink_config.h" + +#include +#include +#include +#include + +// Required by main.h +int cache_vols = 1; +bool reuse_existing_cache = false; + +namespace +{ + +// A compression backend to exercise, along with the RAM_HIT_* state get() +// should report once a compressible object has been stored compressed. +struct CompressionCase { + int config; // CACHE_COMPRESSION_* + int expected_hit; // RAM_HIT_COMPRESS_* reported by get() for compressible data + const char *name; +}; + +std::vector +compression_cases() +{ + std::vector cases{ + {CACHE_COMPRESSION_NONE, RAM_HIT_COMPRESS_NONE, "none" }, + {CACHE_COMPRESSION_FASTLZ, RAM_HIT_COMPRESS_FASTLZ, "fastlz"}, + {CACHE_COMPRESSION_LIBZ, RAM_HIT_COMPRESS_LIBZ, "libz" }, + }; +#ifdef HAVE_LZMA_H + cases.push_back({CACHE_COMPRESSION_LIBLZMA, RAM_HIT_COMPRESS_LIBLZMA, "liblzma"}); +#endif +#ifdef HAVE_LZ4_H + cases.push_back({CACHE_COMPRESSION_LZ4, RAM_HIT_COMPRESS_LZ4, "lz4"}); +#endif + return cases; +} + +// Minimal CacheDisk wiring needed to construct a StripeSM. Mirrors the helper +// in test_Stripe.cc. +void +init_disk(CacheDisk &disk) +{ + disk.path = static_cast(ats_malloc(1)); + disk.path[0] = '\0'; + disk.disk_stripes = static_cast(ats_malloc(sizeof(DiskStripe *))); + disk.disk_stripes[0] = nullptr; + disk.header = static_cast(ats_malloc(sizeof(DiskHeader))); + disk.header->num_volumes = 0; +} + +// The CLFUS get/put/compress paths touch only these metrics and the stripe +// mutex, so that is all the stripe needs for these tests. +void +wire_stripe(StripeSM &stripe, CacheVol &cache_vol) +{ + stripe.cache_vol = &cache_vol; + + cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); + cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); + cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); + cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); + cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); + cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); +} + +Ptr +make_buffer(const std::vector &bytes) +{ + int64_t idx = iobuffer_size_to_index(bytes.size(), MAX_BUFFER_SIZE_INDEX); + Ptr data{make_ptr(new_IOBufferData(idx, MEMALIGNED))}; + std::memcpy(data->data(), bytes.data(), bytes.size()); + return data; +} + +// Highly compressible: a short repeating pattern. +std::vector +compressible_bytes(std::size_t len) +{ + std::vector bytes(len); + for (std::size_t i = 0; i < len; i++) { + bytes[i] = static_cast('A' + (i % 26)); + } + return bytes; +} + +// Effectively incompressible: a deterministic xorshift byte stream. +std::vector +incompressible_bytes(std::size_t len) +{ + std::vector bytes(len); + uint32_t state = 0x9e3779b9; + for (std::size_t i = 0; i < len; i++) { + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + bytes[i] = static_cast(state & 0xff); + } + return bytes; +} + +// Store payload under a fresh key, force a synchronous compression pass with +// `config`, then read it back. Returns the RAM_HIT_* state reported by get() +// and the bytes that were returned. +int +store_compress_get(StripeSM &stripe, int config, const std::vector &payload, std::vector &out) +{ + // Initialize with compression disabled so init() does not schedule the + // background compressor (which would retain a pointer to this stack object). + cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE; + cache_config_ram_cache_compress_percent = 100; + cache_config_ram_cache_use_seen_filter = 0; + + RamCacheCLFUS rc; + rc.init(1 << 20, &stripe); + + Ptr in = make_buffer(payload); + uint32_t len = static_cast(payload.size()); + + static uint64_t salt = 0; + ++salt; + CryptoHash key; + key.u64[0] = 0xc0ffee00 + salt; + key.u64[1] = 0xdeadbeef + salt; + + REQUIRE(rc.put(&key, in.get(), len) == 1); + + cache_config_ram_cache_compress = config; + rc.compress_entries(this_ethread()); + + Ptr ret; + int hit = rc.get(&key, &ret); + REQUIRE(ret.get() != nullptr); + out.assign(ret->data(), ret->data() + len); + return hit; +} + +} // namespace + +TEST_CASE("CLFUS compressible objects roundtrip cleanly", "[cache][ramcache][compress]") +{ + CacheDisk disk; + init_disk(disk); + StripeSM stripe{&disk, 10, 0}; + CacheVol cache_vol; + wire_stripe(stripe, cache_vol); + + auto payload = compressible_bytes(8192); + const CompressionCase c = GENERATE(from_range(compression_cases())); + INFO("compression backend: " << c.name); + + std::vector out; + int hit = store_compress_get(stripe, c.config, payload, out); + + CHECK(hit == c.expected_hit); + CHECK(out == payload); +} + +TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[cache][ramcache][compress]") +{ + CacheDisk disk; + init_disk(disk); + StripeSM stripe{&disk, 10, 0}; + CacheVol cache_vol; + wire_stripe(stripe, cache_vol); + + auto payload = incompressible_bytes(8192); + + // Only the backends that actually attempt compression are interesting here; + // skip the NONE case. + auto cases = compression_cases(); + cases.erase(cases.begin()); + const CompressionCase c = GENERATE_REF(from_range(cases)); + INFO("compression backend: " << c.name); + + std::vector out; + int hit = store_compress_get(stripe, c.config, payload, out); + + // Incompressible data is kept verbatim, so a read reports no compression. + CHECK(hit == RAM_HIT_COMPRESS_NONE); + CHECK(out == payload); +} + +TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") +{ + CacheDisk disk; + init_disk(disk); + StripeSM stripe{&disk, 10, 0}; + CacheVol cache_vol; + wire_stripe(stripe, cache_vol); + + std::vector out; + int hit = store_compress_get(stripe, CACHE_COMPRESSION_NONE, compressible_bytes(1), out); + CHECK(hit == RAM_HIT_COMPRESS_NONE); + CHECK(out == compressible_bytes(1)); +} From f247730f15cecf240ef0f2f7ea81b003c9fab201 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Wed, 10 Jun 2026 23:17:18 +0000 Subject: [PATCH 03/13] Add zstd support --- CMakeLists.txt | 6 ++- cmake/FindZSTD.cmake | 43 +++++++++++++++++++ doc/admin-guide/files/records.yaml.en.rst | 3 +- doc/admin-guide/storage/index.en.rst | 1 + .../cache-architecture/ram-cache.en.rst | 13 +++--- include/iocore/cache/Cache.h | 2 + src/iocore/cache/CMakeLists.txt | 4 ++ src/iocore/cache/CacheProcessor.cc | 5 +++ src/iocore/cache/RamCacheCLFUS.cc | 37 ++++++++++++++++ .../cache/unit_tests/test_RamCacheCLFUS.cc | 3 ++ 10 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 cmake/FindZSTD.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 5ae64050c9d..e3d9b79987e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -489,8 +489,8 @@ set(TS_USE_MALLOC_ALLOCATOR ${ENABLE_MALLOC_ALLOCATOR}) set(TS_USE_ALLOCATOR_METRICS ${ENABLE_ALLOCATOR_METRICS}) find_package(ZLIB REQUIRED) -find_package(zstd CONFIG QUIET) -if(zstd_FOUND) +find_package(ZSTD) +if(ZSTD_FOUND) # Provide a compatibility target name if the upstream package does not export it # Our code links against `zstd::zstd`; upstream zstd usually exports @@ -511,6 +511,8 @@ if(zstd_FOUND) else() set(HAVE_ZSTD_H FALSE) endif() + else() + set(HAVE_ZSTD_H TRUE) endif() else() set(HAVE_ZSTD_H FALSE) diff --git a/cmake/FindZSTD.cmake b/cmake/FindZSTD.cmake new file mode 100644 index 00000000000..737be3b584b --- /dev/null +++ b/cmake/FindZSTD.cmake @@ -0,0 +1,43 @@ +find_path( + ZSTD_INCLUDE_DIR + NAMES zstd.h + DOC "zstd include directory" +) +mark_as_advanced(ZSTD_INCLUDE_DIR) +find_library( + ZSTD_LIBRARY + NAMES zstd libzstd + DOC "zstd library" +) +mark_as_advanced(ZSTD_LIBRARY) + +if(ZSTD_INCLUDE_DIR) + file(STRINGS "${ZSTD_INCLUDE_DIR}/zstd.h" _zstd_version_lines REGEX "#define[ \t]+ZSTD_VERSION_(MAJOR|MINOR|RELEASE)") + string(REGEX REPLACE ".*ZSTD_VERSION_MAJOR *\([0-9]*\).*" "\\1" _zstd_version_major "${_zstd_version_lines}") + string(REGEX REPLACE ".*ZSTD_VERSION_MINOR *\([0-9]*\).*" "\\1" _zstd_version_minor "${_zstd_version_lines}") + string(REGEX REPLACE ".*ZSTD_VERSION_RELEASE *\([0-9]*\).*" "\\1" _zstd_version_release "${_zstd_version_lines}") + set(ZSTD_VERSION "${_zstd_version_major}.${_zstd_version_minor}.${_zstd_version_release}") + unset(_zstd_version_major) + unset(_zstd_version_minor) + unset(_zstd_version_release) + unset(_zstd_version_lines) +endif() + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args( + ZSTD + REQUIRED_VARS ZSTD_LIBRARY ZSTD_INCLUDE_DIR + VERSION_VAR ZSTD_VERSION +) + +if(ZSTD_FOUND) + set(ZSTD_INCLUDE_DIRS "${ZSTD_INCLUDE_DIR}") + set(ZSTD_LIBRARIES "${ZSTD_LIBRARY}") + + if(NOT TARGET zstd::zstd) + add_library(zstd::zstd UNKNOWN IMPORTED) + set_target_properties( + zstd::zstd PROPERTIES IMPORTED_LOCATION "${ZSTD_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${ZSTD_INCLUDE_DIR}" + ) + endif() +endif() diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index cd154186129..66683536b5d 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -3179,9 +3179,10 @@ RAM Cache ======== =================================================================== ``0`` No compression ``1`` Fastlz (extremely fast, relatively low compression) - prefer lz4 - ``2`` Libz (moderate speed, reasonable compression) + ``2`` Libz (moderate speed, reasonable compression) - prefer zstd ``3`` Liblzma (very slow, high compression) ``4`` lz4 (extremely fast, relatively low compression) + ``5`` zstd (fast speed, reasonable compression) ======== =================================================================== Compression runs on task threads. To use more cores for RAM cache diff --git a/doc/admin-guide/storage/index.en.rst b/doc/admin-guide/storage/index.en.rst index 88f9816c754..db29b73618d 100644 --- a/doc/admin-guide/storage/index.en.rst +++ b/doc/admin-guide/storage/index.en.rst @@ -110,6 +110,7 @@ Value Meaning 2 *libz* compression 3 *liblzma* compression 4 *lz4* compression +5 *zstd* compression ======= ============================= .. _changing-the-size-of-the-ram-cache: diff --git a/doc/developer-guide/cache-architecture/ram-cache.en.rst b/doc/developer-guide/cache-architecture/ram-cache.en.rst index 690888df5b3..f557899793c 100644 --- a/doc/developer-guide/cache-architecture/ram-cache.en.rst +++ b/doc/developer-guide/cache-architecture/ram-cache.en.rst @@ -37,7 +37,7 @@ following features: * Is Scan Resistant and extracts robust hit rates even when the working set does not fit in the RAM Cache. -* Supports compression at 4 levels: fastlz, gzip (libz), xz (liblzma) and lz4. +* Supports compression at 5 levels: fastlz, gzip (libz), xz (liblzma), lz4 and zstd. Compression can be moved to another thread. * Has very low CPU overhead, only slightly more than a basic LRU. Rather than @@ -72,7 +72,7 @@ len Length of the object, which differs from *size* because of compression and padding). compressed_len Compressed length of the object. compressed Compression type, or ``none`` if no compression. Possible types - are: *fastlz*, *libz*, *liblzma*, and *lz4*. + are: *fastlz*, *libz*, *liblzma*, *lz4* and *zstd*. uncompressible Flag indicating that content cannot be compressed (true), or that it mat be compressed (false). copy Whether or not this object should be copied in and copied out @@ -147,8 +147,8 @@ since we need to make a copy anyway. Those not tagged ``copy`` are inserted uncompressed in the hope that they can be reused in uncompressed form. This is a compile time option and may be something we want to change. -There are 4 algorithms and levels of compression (speed on an Intel Xeon Gold -6338 processor using lzbench and the silesia XML benchmark): +There are 5 algorithms and levels of compression (speed on an Intel Xeon Gold +6338 processor using lzbench and the silesia XML corpus): ======= ================ ================== ==================================== Method Compression Rate Decompression Rate Notes @@ -156,11 +156,14 @@ Method Compression Rate Decompression Rate Notes fastlz 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. Basically free since disk or network will limit first; ~26% final size. -libz 54 MB/sec 536 MB/sec Almost free, particularly +libz 54 MB/sec 536 MB/sec Effectively obsolete; prefer zstd. + Almost free, particularly decompression; ~13% final size. liblzma 5 MB/sec 291 MB/sec Expensive; ~8% final size. lz4 727 MB/sec 3458 MB/sec Basically free since disk or network will limit first; 23% final size +zstd 508 MB/sec 1690 MB/sec Basically free since disk or network + will limit first; ~12% final size. ======= ================ ================== ==================================== These are ballpark numbers, and your millage will vary enormously. JPEG, for diff --git a/include/iocore/cache/Cache.h b/include/iocore/cache/Cache.h index a4529cb5ac6..115765e5ba9 100644 --- a/include/iocore/cache/Cache.h +++ b/include/iocore/cache/Cache.h @@ -45,6 +45,7 @@ static constexpr ts::ModuleVersion CACHE_MODULE_VERSION(1, 0); #define CACHE_COMPRESSION_LIBZ 2 #define CACHE_COMPRESSION_LIBLZMA 3 #define CACHE_COMPRESSION_LZ4 4 +#define CACHE_COMPRESSION_ZSTD 5 enum { RAM_HIT_COMPRESS_NONE = 1, @@ -52,6 +53,7 @@ enum { RAM_HIT_COMPRESS_LIBZ, RAM_HIT_COMPRESS_LIBLZMA, RAM_HIT_COMPRESS_LZ4, + RAM_HIT_COMPRESS_ZSTD, RAM_HIT_LAST_ENTRY }; diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt index ecdce059bc7..a997dada83f 100644 --- a/src/iocore/cache/CMakeLists.txt +++ b/src/iocore/cache/CMakeLists.txt @@ -62,6 +62,10 @@ if(HAVE_LZ4_H) target_link_libraries(inkcache PRIVATE LZ4::LZ4) endif() +if(HAVE_ZSTD_H) + target_link_libraries(inkcache PRIVATE zstd::zstd) +endif() + if(BUILD_TESTING) # Unit Tests with unit_tests/main.cc macro(add_cache_test name) diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index d21f3c27d64..5a06d22dc2f 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -1682,6 +1682,11 @@ CacheProcessor::cacheInitialized() case CACHE_COMPRESSION_LZ4: #ifndef HAVE_LZ4_H Fatal("lz4 not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_ZSTD: +#ifndef HAVE_ZSTD_H + Fatal("zstd not available for RAM cache compression"); #endif break; } diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 77f7bc349f0..f1a604021ae 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -40,6 +40,9 @@ #ifdef HAVE_LZ4_H #include #endif +#ifdef HAVE_ZSTD_H +#include +#endif // #define CHECK_ACOUNTING 1 // very expensive double checking of all sizes @@ -131,6 +134,11 @@ RamCacheCLFUSCompressor::mainEvent(int /* event ATS_UNUSED */, Event *e) case CACHE_COMPRESSION_LZ4: #ifndef HAVE_LZ4_H Warning("lz4 not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_ZSTD: +#ifndef HAVE_ZSTD_H + Warning("zstd not available for RAM cache compression"); #endif break; } @@ -275,6 +283,18 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey ram_hit_state = RAM_HIT_COMPRESS_LZ4; break; } +#endif +#ifdef HAVE_ZSTD_H + case CACHE_COMPRESSION_ZSTD: { + size_t l = static_cast(e->len); + size_t ll = 0; + ll = ZSTD_decompress(b, l, e->data->data(), e->compressed_len); + if (ZSTD_isError(ll) || l != ll) { + goto Lfailed; + } + ram_hit_state = RAM_HIT_COMPRESS_ZSTD; + break; + } #endif } IOBufferData *data = new_xmalloc_IOBufferData(b, e->len); @@ -453,6 +473,11 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) case CACHE_COMPRESSION_LZ4: l = static_cast(LZ4_compressBound(e->len)); break; +#endif +#ifdef HAVE_ZSTD_H + case CACHE_COMPRESSION_ZSTD: + l = static_cast(ZSTD_compressBound(e->len)); + break; #endif } // store transient data for lock release @@ -501,6 +526,18 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) } break; } +#endif +#ifdef HAVE_ZSTD_H + case CACHE_COMPRESSION_ZSTD: { + size_t ll = l; + size_t zret = ZSTD_compress(b, ll, edata->data(), elen, ZSTD_CLEVEL_DEFAULT); + if (ZSTD_isError(zret)) { + failed = true; + } else { + l = static_cast(zret); + } + break; + } #endif } MUTEX_TAKE_LOCK(stripe->mutex, thread); diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc index 2e6f5dfb4b2..c4c51eadc04 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -62,6 +62,9 @@ compression_cases() #endif #ifdef HAVE_LZ4_H cases.push_back({CACHE_COMPRESSION_LZ4, RAM_HIT_COMPRESS_LZ4, "lz4"}); +#endif +#ifdef HAVE_ZSTD_H + cases.push_back({CACHE_COMPRESSION_ZSTD, RAM_HIT_COMPRESS_ZSTD, "zstd"}); #endif return cases; } From 323d0b3616f2c7c846af7202feaa7751a7f1f5b9 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 00:46:53 +0000 Subject: [PATCH 04/13] Claude PR review remediation --- CMakeLists.txt | 3 +++ ci/docker/yum/Dockerfile | 2 +- src/iocore/cache/RamCacheCLFUS.cc | 7 ++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e3d9b79987e..fbe692a4856 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,6 +495,9 @@ if(ZSTD_FOUND) # Provide a compatibility target name if the upstream package does not export it # Our code links against `zstd::zstd`; upstream zstd usually exports # `zstd::libzstd_shared`/`zstd::libzstd_static`. Create an alias if needed. + # Normally this will be dead code if we use the packaged FindZSTD.cmake; but + # if CMAKE_FIND_PACKAGE_PREFER_CONFIG=1 and zstd-config.cmake is found, this + # may be useful. if(NOT TARGET zstd::zstd) if(TARGET zstd::libzstd_shared) set(_zstd_target zstd::libzstd_shared) diff --git a/ci/docker/yum/Dockerfile b/ci/docker/yum/Dockerfile index 4f53cadea60..c97237b21ab 100644 --- a/ci/docker/yum/Dockerfile +++ b/ci/docker/yum/Dockerfile @@ -52,7 +52,7 @@ RUN yum -y update; \ # Devel packages that ATS needs yum -y install openssl-devel expat-devel pcre-devel libcap-devel hwloc-devel libunwind-devel \ xz-devel libcurl-devel ncurses-devel jemalloc-devel GeoIP-devel luajit-devel brotli-devel \ - ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel zstd-devel \ + ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel zstd-devel lz4-devel \ perl-ExtUtils-MakeMaker perl-Digest-SHA perl-URI; \ # This is for autest stuff yum -y install python3 httpd-tools procps-ng nmap-ncat \ diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index f1a604021ae..c27ce6afd91 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -42,6 +42,7 @@ #endif #ifdef HAVE_ZSTD_H #include +constexpr int CLFUS_ZSTD_LEVEL = 3; #endif // #define CHECK_ACOUNTING 1 // very expensive double checking of all sizes @@ -122,9 +123,7 @@ RamCacheCLFUSCompressor::mainEvent(int /* event ATS_UNUSED */, Event *e) Warning("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); case CACHE_COMPRESSION_NONE: case CACHE_COMPRESSION_FASTLZ: - break; case CACHE_COMPRESSION_LIBZ: - Warning("libz not available for RAM cache compression"); break; case CACHE_COMPRESSION_LIBLZMA: #ifndef HAVE_LZMA_H @@ -288,6 +287,7 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey case CACHE_COMPRESSION_ZSTD: { size_t l = static_cast(e->len); size_t ll = 0; + // TODO: Use a thread_local ZSTD_DCtx ll = ZSTD_decompress(b, l, e->data->data(), e->compressed_len); if (ZSTD_isError(ll) || l != ll) { goto Lfailed; @@ -530,7 +530,8 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) #ifdef HAVE_ZSTD_H case CACHE_COMPRESSION_ZSTD: { size_t ll = l; - size_t zret = ZSTD_compress(b, ll, edata->data(), elen, ZSTD_CLEVEL_DEFAULT); + // TODO: Use a thread_local ZSTD_CCtx + size_t zret = ZSTD_compress(b, ll, edata->data(), elen, CLFUS_ZSTD_LEVEL); if (ZSTD_isError(zret)) { failed = true; } else { From d73eb5658400196b4c060151157300b1438f2640 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 01:49:15 +0000 Subject: [PATCH 05/13] Reuse zstd context --- CMakeLists.txt | 4 ++- src/iocore/cache/RamCacheCLFUS.cc | 51 ++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fbe692a4856..e045f6827ca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -489,7 +489,9 @@ set(TS_USE_MALLOC_ALLOCATOR ${ENABLE_MALLOC_ALLOCATOR}) set(TS_USE_ALLOCATOR_METRICS ${ENABLE_ALLOCATOR_METRICS}) find_package(ZLIB REQUIRED) -find_package(ZSTD) +# 1.4.0 stabilized the advanced one-shot API (ZSTD_compress2 et al.) used by +# the RAM cache. +find_package(ZSTD 1.4.0) if(ZSTD_FOUND) # Provide a compatibility target name if the upstream package does not export it diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index c27ce6afd91..0cd0d498142 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -42,7 +42,39 @@ #endif #ifdef HAVE_ZSTD_H #include +#include constexpr int CLFUS_ZSTD_LEVEL = 3; + +namespace +{ + +// One-shot ZSTD_compress/ZSTD_decompress allocate and free a context on every +// call, so reuse a per-thread context instead. May return nullptr if zstd +// fails to allocate one. The compression level is a sticky parameter set once +// here; no explicit ZSTD_CCtx_reset() is needed because ZSTD_compress2() +// starts a new session on every call (resets are only for interrupting the +// streaming API or changing sticky parameters). +ZSTD_CCtx * +zstd_cctx() +{ + thread_local std::unique_ptr ctx = [] { + std::unique_ptr c{ZSTD_createCCtx(), ZSTD_freeCCtx}; + if (c && ZSTD_isError(ZSTD_CCtx_setParameter(c.get(), ZSTD_c_compressionLevel, CLFUS_ZSTD_LEVEL))) { + c.reset(); + } + return c; + }(); + return ctx.get(); +} + +ZSTD_DCtx * +zstd_dctx() +{ + thread_local std::unique_ptr ctx{ZSTD_createDCtx(), ZSTD_freeDCtx}; + return ctx.get(); +} + +} // end anonymous namespace #endif // #define CHECK_ACOUNTING 1 // very expensive double checking of all sizes @@ -285,10 +317,12 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey #endif #ifdef HAVE_ZSTD_H case CACHE_COMPRESSION_ZSTD: { - size_t l = static_cast(e->len); - size_t ll = 0; - // TODO: Use a thread_local ZSTD_DCtx - ll = ZSTD_decompress(b, l, e->data->data(), e->compressed_len); + size_t l = static_cast(e->len); + ZSTD_DCtx *dctx = zstd_dctx(); + if (dctx == nullptr) { + goto Lfailed; + } + size_t ll = ZSTD_decompressDCtx(dctx, b, l, e->data->data(), e->compressed_len); if (ZSTD_isError(ll) || l != ll) { goto Lfailed; } @@ -529,9 +563,12 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) #endif #ifdef HAVE_ZSTD_H case CACHE_COMPRESSION_ZSTD: { - size_t ll = l; - // TODO: Use a thread_local ZSTD_CCtx - size_t zret = ZSTD_compress(b, ll, edata->data(), elen, CLFUS_ZSTD_LEVEL); + ZSTD_CCtx *cctx = zstd_cctx(); + if (cctx == nullptr) { + failed = true; + break; + } + size_t zret = ZSTD_compress2(cctx, b, l, edata->data(), elen); if (ZSTD_isError(zret)) { failed = true; } else { From 7ed2d6cf7d1a434e4c8ed38da1a3bcdb1696e3dc Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 02:46:39 +0000 Subject: [PATCH 06/13] Add licenses to CMake files --- NOTICE | 7 +++++++ cmake/FindLZ4.cmake | 35 +++++++++++++++++++++++++++++++++++ cmake/FindZSTD.cmake | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/NOTICE b/NOTICE index 7fb1e7a6358..aa33c330c70 100644 --- a/NOTICE +++ b/NOTICE @@ -149,3 +149,10 @@ algorithms (Wojciech Muła and Daniel Lemire's vectorized base64, and aqrit's combined standard/URL-safe classifier) and lookup tables originate there. simdutf: https://github.com/simdutf/simdutf (Apache-2.0 / MIT / BSL-1.0) Copyright (c) 2021 The simdutf authors + +~~ + +cmake/FindLZ4.cmake and cmake/FindZSTD.cmake derived from: +VTK: open-source software system for image processing, 3D graphics, volume rendering and visualization +Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen (BSD-3-Clause License) +https://gitlab.kitware.com/vtk/vtk diff --git a/cmake/FindLZ4.cmake b/cmake/FindLZ4.cmake index c36a6573c32..1c53b1bf121 100644 --- a/cmake/FindLZ4.cmake +++ b/cmake/FindLZ4.cmake @@ -1,3 +1,38 @@ +#========================================================================= +# +# Sourced from the Visualization Toolkit (VTK), CMake/FindLZ4.cmake: +# https://gitlab.kitware.com/vtk/vtk +# +# Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither name of Ken Martin, Will Schroeder, or Bill Lorensen nor the names +# of any contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +#========================================================================= + find_path( LZ4_INCLUDE_DIR NAMES lz4.h diff --git a/cmake/FindZSTD.cmake b/cmake/FindZSTD.cmake index 737be3b584b..9634a96a401 100644 --- a/cmake/FindZSTD.cmake +++ b/cmake/FindZSTD.cmake @@ -1,3 +1,38 @@ +#========================================================================= +# +# Derived from the Visualization Toolkit (VTK), CMake/FindLZ4.cmake: +# https://gitlab.kitware.com/vtk/vtk +# +# Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither name of Ken Martin, Will Schroeder, or Bill Lorensen nor the names +# of any contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +#========================================================================= + find_path( ZSTD_INCLUDE_DIR NAMES zstd.h From 464ecd6c74b7f9a45aa8490fc718b1e4e77b1a90 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 21:22:06 +0000 Subject: [PATCH 07/13] PR remediations --- ci/rat-exclude.txt | 2 + .../statistics/core/cache-volume.en.rst | 5 ++ .../monitoring/statistics/core/cache.en.rst | 5 ++ .../cache-architecture/ram-cache.en.rst | 32 +++---- include/iocore/cache/Cache.h | 9 ++ src/iocore/cache/CacheProcessor.cc | 75 ++++++++-------- src/iocore/cache/P_CacheStats.h | 75 ++++++++-------- src/iocore/cache/RamCacheCLFUS.cc | 40 ++++++++- .../cache/unit_tests/test_RamCacheCLFUS.cc | 90 +++++++++++++------ 9 files changed, 213 insertions(+), 120 deletions(-) diff --git a/ci/rat-exclude.txt b/ci/rat-exclude.txt index a3271fb6a38..c9e01c3c81e 100644 --- a/ci/rat-exclude.txt +++ b/ci/rat-exclude.txt @@ -82,3 +82,5 @@ tools/http_load/** **/clang-tidy.conf build*/** cmake-build*/** +cmake/FindLZ4.cmake +cmake/FindZSTD.cmake diff --git a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst index c6e39eb7449..eb6b8b2c512 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst @@ -132,6 +132,11 @@ a configuration with only one cache volume: :literal:`0`. Accumulates the number of misses to the LRU RAM cache for this volume. Note that this count includes hits to the other memory caches, including the last open read and aggregation buffer caches, so it may not represent the total number of cache accesses that go to disk. +.. ts:stat:: global proxy.process.cache.volume_0.ram_cache.decompress.failure integer + :type: counter + + Accumulates the number of RAM cache entries that failed to decompress on read, for this volume. A failed entry is dropped from the RAM cache and the read is treated as a miss. A nonzero value indicates data corruption or a compression library error, not ordinary cache churn. + .. ts:stat:: global proxy.process.cache.volume_0.last_open_read.hits integer :type: counter diff --git a/doc/admin-guide/monitoring/statistics/core/cache.en.rst b/doc/admin-guide/monitoring/statistics/core/cache.en.rst index bb2f7d17f42..b06bb9802a5 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache.en.rst @@ -95,6 +95,11 @@ Cache Accumulates the number of misses to the LRU RAM cache for all volumes. Note that this includes hits to the other memory caches, including the last open read and aggregation buffer caches, so it may not represent the total number of cache accesses that go to disk. +.. ts:stat:: global proxy.process.cache.ram_cache.decompress.failure integer + :type: counter + + Accumulates the number of RAM cache entries that failed to decompress on read, for all volumes. A failed entry is dropped from the RAM cache and the read is treated as a miss. A nonzero value indicates data corruption or a compression library error, not ordinary cache churn. + .. ts:stat:: global proxy.process.cache.last_open_read.hits integer :type: counter diff --git a/doc/developer-guide/cache-architecture/ram-cache.en.rst b/doc/developer-guide/cache-architecture/ram-cache.en.rst index f557899793c..2fa9c2caa46 100644 --- a/doc/developer-guide/cache-architecture/ram-cache.en.rst +++ b/doc/developer-guide/cache-architecture/ram-cache.en.rst @@ -73,7 +73,7 @@ len Length of the object, which differs from *size* because of compressed_len Compressed length of the object. compressed Compression type, or ``none`` if no compression. Possible types are: *fastlz*, *libz*, *liblzma*, *lz4* and *zstd*. -uncompressible Flag indicating that content cannot be compressed (true), or that +incompressible Flag indicating that content cannot be compressed (true), or that it mat be compressed (false). copy Whether or not this object should be copied in and copied out (e.g. HTTP HDR). @@ -150,21 +150,21 @@ a compile time option and may be something we want to change. There are 5 algorithms and levels of compression (speed on an Intel Xeon Gold 6338 processor using lzbench and the silesia XML corpus): -======= ================ ================== ==================================== -Method Compression Rate Decompression Rate Notes -======= ================ ================== ==================================== -fastlz 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. - Basically free since disk or network - will limit first; ~26% final size. -libz 54 MB/sec 536 MB/sec Effectively obsolete; prefer zstd. - Almost free, particularly - decompression; ~13% final size. -liblzma 5 MB/sec 291 MB/sec Expensive; ~8% final size. -lz4 727 MB/sec 3458 MB/sec Basically free since disk or network - will limit first; 23% final size -zstd 508 MB/sec 1690 MB/sec Basically free since disk or network - will limit first; ~12% final size. -======= ================ ================== ==================================== +======= ===== ================= ================== ==================================== +Method Level Compression Rate Decompression Rate Notes +======= ===== ================= ================== ==================================== +fastlz 1 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. + Basically free since disk or network + will limit first; ~26% final size. +libz 6 54 MB/sec 536 MB/sec Effectively obsolete; prefer zstd. + Almost free, particularly + decompression; ~13% final size. +liblzma 6 5 MB/sec 291 MB/sec Expensive; ~8% final size. +lz4 1 727 MB/sec 3458 MB/sec Basically free since disk or network + will limit first; ~23% final size. +zstd 3 508 MB/sec 1690 MB/sec Basically free since disk or network + will limit first; ~12% final size. +======= ===== ================= ================== ==================================== These are ballpark numbers, and your millage will vary enormously. JPEG, for example, will not compress with any of these (or at least will only do so at diff --git a/include/iocore/cache/Cache.h b/include/iocore/cache/Cache.h index 115765e5ba9..fd23132f063 100644 --- a/include/iocore/cache/Cache.h +++ b/include/iocore/cache/Cache.h @@ -57,6 +57,15 @@ enum { RAM_HIT_LAST_ENTRY }; +// The RAM_HIT_COMPRESS_* values are the CACHE_COMPRESSION_* values offset by +// one; keep the two sequences from silently desyncing when a codec is added. +static_assert(RAM_HIT_COMPRESS_NONE == CACHE_COMPRESSION_NONE + 1); +static_assert(RAM_HIT_COMPRESS_FASTLZ == CACHE_COMPRESSION_FASTLZ + 1); +static_assert(RAM_HIT_COMPRESS_LIBZ == CACHE_COMPRESSION_LIBZ + 1); +static_assert(RAM_HIT_COMPRESS_LIBLZMA == CACHE_COMPRESSION_LIBLZMA + 1); +static_assert(RAM_HIT_COMPRESS_LZ4 == CACHE_COMPRESSION_LZ4 + 1); +static_assert(RAM_HIT_COMPRESS_ZSTD == CACHE_COMPRESSION_ZSTD + 1); + struct CacheVC; class CacheEvacuateDocVC; struct CacheDisk; diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index 5a06d22dc2f..15972b1dcac 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -1183,43 +1183,44 @@ register_cache_stats(CacheStatsBlock *rsb, const std::string &prefix) rsb->fragment_document_count[2] = ts::Metrics::Counter::createPtr(prefix + ".frags_per_doc.3+"); // And then everything else - rsb->bytes_used = ts::Metrics::Gauge::createPtr(prefix + ".bytes_used"); - rsb->bytes_total = ts::Metrics::Gauge::createPtr(prefix + ".bytes_total"); - rsb->stripes = ts::Metrics::Gauge::createPtr(prefix + ".stripes"); - rsb->ram_cache_bytes_total = ts::Metrics::Gauge::createPtr(prefix + ".ram_cache.total_bytes"); - rsb->ram_cache_bytes = ts::Metrics::Gauge::createPtr(prefix + ".ram_cache.bytes_used"); - rsb->ram_cache_hits = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.hits"); - rsb->last_open_read_hits = ts::Metrics::Counter::createPtr(prefix + ".last_open_read.hits"); - rsb->agg_buffer_hits = ts::Metrics::Counter::createPtr(prefix + ".aggregation_buffer.hits"); - rsb->ram_cache_misses = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.misses"); - rsb->all_mem_misses = ts::Metrics::Counter::createPtr(prefix + ".all_memory_caches.misses"); - rsb->pread_count = ts::Metrics::Counter::createPtr(prefix + ".pread_count"); - rsb->percent_full = ts::Metrics::Gauge::createPtr(prefix + ".percent_full"); - rsb->read_seek_fail = ts::Metrics::Counter::createPtr(prefix + ".read.seek.failure"); - rsb->read_invalid = ts::Metrics::Counter::createPtr(prefix + ".read.invalid"); - rsb->write_backlog_failure = ts::Metrics::Counter::createPtr(prefix + ".write.backlog.failure"); - rsb->direntries_total = ts::Metrics::Gauge::createPtr(prefix + ".direntries.total"); - rsb->direntries_used = ts::Metrics::Gauge::createPtr(prefix + ".direntries.used"); - rsb->directory_collision = ts::Metrics::Counter::createPtr(prefix + ".directory_collision"); - rsb->read_busy_success = ts::Metrics::Counter::createPtr(prefix + ".read_busy.success"); - rsb->read_busy_failure = ts::Metrics::Counter::createPtr(prefix + ".read_busy.failure"); - rsb->write_bytes = ts::Metrics::Counter::createPtr(prefix + ".write_bytes_stat"); - rsb->hdr_vector_marshal = ts::Metrics::Counter::createPtr(prefix + ".vector_marshals"); - rsb->hdr_marshal = ts::Metrics::Counter::createPtr(prefix + ".hdr_marshals"); - rsb->hdr_marshal_bytes = ts::Metrics::Counter::createPtr(prefix + ".hdr_marshal_bytes"); - rsb->gc_bytes_evacuated = ts::Metrics::Counter::createPtr(prefix + ".gc_bytes_evacuated"); - rsb->gc_frags_evacuated = ts::Metrics::Counter::createPtr(prefix + ".gc_frags_evacuated"); - rsb->directory_wrap = ts::Metrics::Counter::createPtr(prefix + ".wrap_count"); - rsb->directory_sync_count = ts::Metrics::Counter::createPtr(prefix + ".sync.count"); - rsb->directory_sync_bytes = ts::Metrics::Counter::createPtr(prefix + ".sync.bytes"); - rsb->directory_sync_time = ts::Metrics::Counter::createPtr(prefix + ".sync.time"); - rsb->span_errors_read = ts::Metrics::Counter::createPtr(prefix + ".span.errors.read"); - rsb->span_errors_write = ts::Metrics::Counter::createPtr(prefix + ".span.errors.write"); - rsb->span_failing = ts::Metrics::Gauge::createPtr(prefix + ".span.failing"); - rsb->span_offline = ts::Metrics::Gauge::createPtr(prefix + ".span.offline"); - rsb->span_online = ts::Metrics::Gauge::createPtr(prefix + ".span.online"); - rsb->stripe_lock_contention = ts::Metrics::Counter::createPtr(prefix + ".stripe.lock_contention"); - rsb->writer_lock_contention = ts::Metrics::Counter::createPtr(prefix + ".writer.lock_contention"); + rsb->bytes_used = ts::Metrics::Gauge::createPtr(prefix + ".bytes_used"); + rsb->bytes_total = ts::Metrics::Gauge::createPtr(prefix + ".bytes_total"); + rsb->stripes = ts::Metrics::Gauge::createPtr(prefix + ".stripes"); + rsb->ram_cache_bytes_total = ts::Metrics::Gauge::createPtr(prefix + ".ram_cache.total_bytes"); + rsb->ram_cache_bytes = ts::Metrics::Gauge::createPtr(prefix + ".ram_cache.bytes_used"); + rsb->ram_cache_hits = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.hits"); + rsb->last_open_read_hits = ts::Metrics::Counter::createPtr(prefix + ".last_open_read.hits"); + rsb->agg_buffer_hits = ts::Metrics::Counter::createPtr(prefix + ".aggregation_buffer.hits"); + rsb->ram_cache_misses = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.misses"); + rsb->ram_cache_decompress_failures = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.decompress.failure"); + rsb->all_mem_misses = ts::Metrics::Counter::createPtr(prefix + ".all_memory_caches.misses"); + rsb->pread_count = ts::Metrics::Counter::createPtr(prefix + ".pread_count"); + rsb->percent_full = ts::Metrics::Gauge::createPtr(prefix + ".percent_full"); + rsb->read_seek_fail = ts::Metrics::Counter::createPtr(prefix + ".read.seek.failure"); + rsb->read_invalid = ts::Metrics::Counter::createPtr(prefix + ".read.invalid"); + rsb->write_backlog_failure = ts::Metrics::Counter::createPtr(prefix + ".write.backlog.failure"); + rsb->direntries_total = ts::Metrics::Gauge::createPtr(prefix + ".direntries.total"); + rsb->direntries_used = ts::Metrics::Gauge::createPtr(prefix + ".direntries.used"); + rsb->directory_collision = ts::Metrics::Counter::createPtr(prefix + ".directory_collision"); + rsb->read_busy_success = ts::Metrics::Counter::createPtr(prefix + ".read_busy.success"); + rsb->read_busy_failure = ts::Metrics::Counter::createPtr(prefix + ".read_busy.failure"); + rsb->write_bytes = ts::Metrics::Counter::createPtr(prefix + ".write_bytes_stat"); + rsb->hdr_vector_marshal = ts::Metrics::Counter::createPtr(prefix + ".vector_marshals"); + rsb->hdr_marshal = ts::Metrics::Counter::createPtr(prefix + ".hdr_marshals"); + rsb->hdr_marshal_bytes = ts::Metrics::Counter::createPtr(prefix + ".hdr_marshal_bytes"); + rsb->gc_bytes_evacuated = ts::Metrics::Counter::createPtr(prefix + ".gc_bytes_evacuated"); + rsb->gc_frags_evacuated = ts::Metrics::Counter::createPtr(prefix + ".gc_frags_evacuated"); + rsb->directory_wrap = ts::Metrics::Counter::createPtr(prefix + ".wrap_count"); + rsb->directory_sync_count = ts::Metrics::Counter::createPtr(prefix + ".sync.count"); + rsb->directory_sync_bytes = ts::Metrics::Counter::createPtr(prefix + ".sync.bytes"); + rsb->directory_sync_time = ts::Metrics::Counter::createPtr(prefix + ".sync.time"); + rsb->span_errors_read = ts::Metrics::Counter::createPtr(prefix + ".span.errors.read"); + rsb->span_errors_write = ts::Metrics::Counter::createPtr(prefix + ".span.errors.write"); + rsb->span_failing = ts::Metrics::Gauge::createPtr(prefix + ".span.failing"); + rsb->span_offline = ts::Metrics::Gauge::createPtr(prefix + ".span.offline"); + rsb->span_online = ts::Metrics::Gauge::createPtr(prefix + ".span.online"); + rsb->stripe_lock_contention = ts::Metrics::Counter::createPtr(prefix + ".stripe.lock_contention"); + rsb->writer_lock_contention = ts::Metrics::Counter::createPtr(prefix + ".writer.lock_contention"); } // Copy the per-volume tuning fields from the volume config onto the CacheVol. diff --git a/src/iocore/cache/P_CacheStats.h b/src/iocore/cache/P_CacheStats.h index 7673cf56e0e..08120605b47 100644 --- a/src/iocore/cache/P_CacheStats.h +++ b/src/iocore/cache/P_CacheStats.h @@ -37,41 +37,42 @@ struct CacheStatsBlock { ts::Metrics::Counter::AtomicType *fragment_document_count[3] = {nullptr, nullptr, nullptr}; // For 1, 2 and 3+ fragments - ts::Metrics::Gauge::AtomicType *bytes_used = nullptr; - ts::Metrics::Gauge::AtomicType *bytes_total = nullptr; - ts::Metrics::Gauge::AtomicType *stripes = nullptr; - ts::Metrics::Gauge::AtomicType *ram_cache_bytes = nullptr; - ts::Metrics::Gauge::AtomicType *ram_cache_bytes_total = nullptr; - ts::Metrics::Gauge::AtomicType *direntries_total = nullptr; - ts::Metrics::Gauge::AtomicType *direntries_used = nullptr; - ts::Metrics::Counter::AtomicType *ram_cache_hits = nullptr; - ts::Metrics::Counter::AtomicType *last_open_read_hits = nullptr; - ts::Metrics::Counter::AtomicType *agg_buffer_hits = nullptr; - ts::Metrics::Counter::AtomicType *ram_cache_misses = nullptr; - ts::Metrics::Counter::AtomicType *all_mem_misses = nullptr; - ts::Metrics::Counter::AtomicType *pread_count = nullptr; - ts::Metrics::Gauge::AtomicType *percent_full = nullptr; - ts::Metrics::Counter::AtomicType *read_seek_fail = nullptr; - ts::Metrics::Counter::AtomicType *read_invalid = nullptr; - ts::Metrics::Counter::AtomicType *write_backlog_failure = nullptr; - ts::Metrics::Counter::AtomicType *directory_collision = nullptr; - ts::Metrics::Counter::AtomicType *read_busy_success = nullptr; - ts::Metrics::Counter::AtomicType *read_busy_failure = nullptr; - ts::Metrics::Counter::AtomicType *gc_bytes_evacuated = nullptr; - ts::Metrics::Counter::AtomicType *gc_frags_evacuated = nullptr; - ts::Metrics::Counter::AtomicType *write_bytes = nullptr; - ts::Metrics::Counter::AtomicType *hdr_vector_marshal = nullptr; - ts::Metrics::Counter::AtomicType *hdr_marshal = nullptr; - ts::Metrics::Counter::AtomicType *hdr_marshal_bytes = nullptr; - ts::Metrics::Counter::AtomicType *directory_wrap = nullptr; - ts::Metrics::Counter::AtomicType *directory_sync_count = nullptr; - ts::Metrics::Counter::AtomicType *directory_sync_time = nullptr; - ts::Metrics::Counter::AtomicType *directory_sync_bytes = nullptr; - ts::Metrics::Counter::AtomicType *span_errors_read = nullptr; - ts::Metrics::Counter::AtomicType *span_errors_write = nullptr; - ts::Metrics::Gauge::AtomicType *span_offline = nullptr; - ts::Metrics::Gauge::AtomicType *span_online = nullptr; - ts::Metrics::Gauge::AtomicType *span_failing = nullptr; - ts::Metrics::Counter::AtomicType *stripe_lock_contention = nullptr; - ts::Metrics::Counter::AtomicType *writer_lock_contention = nullptr; + ts::Metrics::Gauge::AtomicType *bytes_used = nullptr; + ts::Metrics::Gauge::AtomicType *bytes_total = nullptr; + ts::Metrics::Gauge::AtomicType *stripes = nullptr; + ts::Metrics::Gauge::AtomicType *ram_cache_bytes = nullptr; + ts::Metrics::Gauge::AtomicType *ram_cache_bytes_total = nullptr; + ts::Metrics::Gauge::AtomicType *direntries_total = nullptr; + ts::Metrics::Gauge::AtomicType *direntries_used = nullptr; + ts::Metrics::Counter::AtomicType *ram_cache_hits = nullptr; + ts::Metrics::Counter::AtomicType *last_open_read_hits = nullptr; + ts::Metrics::Counter::AtomicType *agg_buffer_hits = nullptr; + ts::Metrics::Counter::AtomicType *ram_cache_misses = nullptr; + ts::Metrics::Counter::AtomicType *ram_cache_decompress_failures = nullptr; + ts::Metrics::Counter::AtomicType *all_mem_misses = nullptr; + ts::Metrics::Counter::AtomicType *pread_count = nullptr; + ts::Metrics::Gauge::AtomicType *percent_full = nullptr; + ts::Metrics::Counter::AtomicType *read_seek_fail = nullptr; + ts::Metrics::Counter::AtomicType *read_invalid = nullptr; + ts::Metrics::Counter::AtomicType *write_backlog_failure = nullptr; + ts::Metrics::Counter::AtomicType *directory_collision = nullptr; + ts::Metrics::Counter::AtomicType *read_busy_success = nullptr; + ts::Metrics::Counter::AtomicType *read_busy_failure = nullptr; + ts::Metrics::Counter::AtomicType *gc_bytes_evacuated = nullptr; + ts::Metrics::Counter::AtomicType *gc_frags_evacuated = nullptr; + ts::Metrics::Counter::AtomicType *write_bytes = nullptr; + ts::Metrics::Counter::AtomicType *hdr_vector_marshal = nullptr; + ts::Metrics::Counter::AtomicType *hdr_marshal = nullptr; + ts::Metrics::Counter::AtomicType *hdr_marshal_bytes = nullptr; + ts::Metrics::Counter::AtomicType *directory_wrap = nullptr; + ts::Metrics::Counter::AtomicType *directory_sync_count = nullptr; + ts::Metrics::Counter::AtomicType *directory_sync_time = nullptr; + ts::Metrics::Counter::AtomicType *directory_sync_bytes = nullptr; + ts::Metrics::Counter::AtomicType *span_errors_read = nullptr; + ts::Metrics::Counter::AtomicType *span_errors_write = nullptr; + ts::Metrics::Gauge::AtomicType *span_offline = nullptr; + ts::Metrics::Gauge::AtomicType *span_online = nullptr; + ts::Metrics::Gauge::AtomicType *span_failing = nullptr; + ts::Metrics::Counter::AtomicType *stripe_lock_contention = nullptr; + ts::Metrics::Counter::AtomicType *writer_lock_contention = nullptr; }; diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 0cd0d498142..39cdbbc1809 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -33,6 +33,9 @@ #include "fastlz/fastlz.h" #include "tscore/CryptoHash.h" #include "tscore/Regression.h" +#include "tscore/Throttler.h" + +#include #include #ifdef HAVE_LZMA_H #include @@ -45,12 +48,17 @@ #include constexpr int CLFUS_ZSTD_LEVEL = 3; +// The compression type is stored in the 3-bit RamCacheCLFUSEntry +// flag_bits.compressed field; a new codec value must still fit. +static_assert(CACHE_COMPRESSION_ZSTD < (1 << 3)); + namespace { // One-shot ZSTD_compress/ZSTD_decompress allocate and free a context on every // call, so reuse a per-thread context instead. May return nullptr if zstd -// fails to allocate one. The compression level is a sticky parameter set once +// fails to allocate one; that failure is sticky for the life of the thread, so +// warn when it happens. The compression level is a sticky parameter set once // here; no explicit ZSTD_CCtx_reset() is needed because ZSTD_compress2() // starts a new session on every call (resets are only for interrupting the // streaming API or changing sticky parameters). @@ -62,6 +70,9 @@ zstd_cctx() if (c && ZSTD_isError(ZSTD_CCtx_setParameter(c.get(), ZSTD_c_compressionLevel, CLFUS_ZSTD_LEVEL))) { c.reset(); } + if (!c) { + Warning("unable to allocate zstd compression context; RAM cache entries will not be compressed on this thread"); + } return c; }(); return ctx.get(); @@ -70,7 +81,13 @@ zstd_cctx() ZSTD_DCtx * zstd_dctx() { - thread_local std::unique_ptr ctx{ZSTD_createDCtx(), ZSTD_freeDCtx}; + thread_local std::unique_ptr ctx = [] { + std::unique_ptr c{ZSTD_createDCtx(), ZSTD_freeDCtx}; + if (!c) { + Warning("unable to allocate zstd decompression context; compressed RAM cache entries will miss on this thread"); + } + return c; + }(); return ctx.get(); } @@ -320,7 +337,10 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey size_t l = static_cast(e->len); ZSTD_DCtx *dctx = zstd_dctx(); if (dctx == nullptr) { - goto Lfailed; + // This thread can't decompress, but the entry itself is fine: + // miss instead of evicting it. + ats_free(b); + goto Lerror; } size_t ll = ZSTD_decompressDCtx(dctx, b, l, e->data->data(), e->compressed_len); if (ZSTD_isError(ll) || l != ll) { @@ -374,6 +394,20 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey return 0; Lfailed: ats_free(b); + { + // A failure here is data corruption or a codec error, not an ordinary + // miss; make it visible beyond the debug-gated trace below. + static Throttler decompress_failure_throttler(std::chrono::seconds(60)); + + uint64_t suppressed = 0; + if (!decompress_failure_throttler.is_throttled(suppressed)) { + Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X; entry dropped" + " (%" PRIu64 " similar failures suppressed)", + static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), suppressed); + } + ts::Metrics::Counter::increment(cache_rsb.ram_cache_decompress_failures); + ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_decompress_failures); + } this->_destroy(e); DDbg(dbg_ctl_ram_cache, "get %X %" PRId64 " Z_ERR", key->slice32(3), auxkey); goto Lerror; diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc index c4c51eadc04..cb234e682df 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -89,12 +89,15 @@ wire_stripe(StripeSM &stripe, CacheVol &cache_vol) { stripe.cache_vol = &cache_vol; - cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); - cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); - cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); - cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); - cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); - cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); + cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); + cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); + cache_rsb.ram_cache_decompress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.decompress.failure"); + cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); + cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); + cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_vol.vol_rsb.ram_cache_decompress_failures = + ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.decompress.failure"); } Ptr @@ -132,11 +135,17 @@ incompressible_bytes(std::size_t len) return bytes; } +struct RoundtripResult { + int hit = 0; + int64_t size_before = 0; // rc.size() after put, before the compression pass + int64_t size_after = 0; // rc.size() after the compression pass + std::vector out; +}; + // Store payload under a fresh key, force a synchronous compression pass with -// `config`, then read it back. Returns the RAM_HIT_* state reported by get() -// and the bytes that were returned. -int -store_compress_get(StripeSM &stripe, int config, const std::vector &payload, std::vector &out) +// `config`, then read it back. +RoundtripResult +store_compress_get(StripeSM &stripe, int config, const std::vector &payload) { // Initialize with compression disabled so init() does not schedule the // background compressor (which would retain a pointer to this stack object). @@ -158,14 +167,19 @@ store_compress_get(StripeSM &stripe, int config, const std::vector &payloa REQUIRE(rc.put(&key, in.get(), len) == 1); + RoundtripResult r; + + r.size_before = rc.size(); cache_config_ram_cache_compress = config; rc.compress_entries(this_ethread()); + r.size_after = rc.size(); Ptr ret; - int hit = rc.get(&key, &ret); + + r.hit = rc.get(&key, &ret); REQUIRE(ret.get() != nullptr); - out.assign(ret->data(), ret->data() + len); - return hit; + r.out.assign(ret->data(), ret->data() + len); + return r; } } // namespace @@ -178,15 +192,21 @@ TEST_CASE("CLFUS compressible objects roundtrip cleanly", "[cache][ramcache][com CacheVol cache_vol; wire_stripe(stripe, cache_vol); - auto payload = compressible_bytes(8192); + // Large enough to exercise the *_compressBound() arithmetic and uint32_t + // casts in compress_entries(), not just small-buffer paths. + auto payload = compressible_bytes(256 * 1024); const CompressionCase c = GENERATE(from_range(compression_cases())); INFO("compression backend: " << c.name); - std::vector out; - int hit = store_compress_get(stripe, c.config, payload, out); + RoundtripResult r = store_compress_get(stripe, c.config, payload); - CHECK(hit == c.expected_hit); - CHECK(out == payload); + CHECK(r.hit == c.expected_hit); + CHECK(r.out == payload); + if (c.config != CACHE_COMPRESSION_NONE) { + // The feature's contract is that compression saves memory, not merely + // that the entry is tagged compressed. + CHECK(r.size_after < r.size_before); + } } TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[cache][ramcache][compress]") @@ -197,7 +217,7 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca CacheVol cache_vol; wire_stripe(stripe, cache_vol); - auto payload = incompressible_bytes(8192); + auto payload = incompressible_bytes(256 * 1024); // Only the backends that actually attempt compression are interesting here; // skip the NONE case. @@ -206,12 +226,11 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca const CompressionCase c = GENERATE_REF(from_range(cases)); INFO("compression backend: " << c.name); - std::vector out; - int hit = store_compress_get(stripe, c.config, payload, out); + RoundtripResult r = store_compress_get(stripe, c.config, payload); // Incompressible data is kept verbatim, so a read reports no compression. - CHECK(hit == RAM_HIT_COMPRESS_NONE); - CHECK(out == payload); + CHECK(r.hit == RAM_HIT_COMPRESS_NONE); + CHECK(r.out == payload); } TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") @@ -222,8 +241,25 @@ TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") CacheVol cache_vol; wire_stripe(stripe, cache_vol); - std::vector out; - int hit = store_compress_get(stripe, CACHE_COMPRESSION_NONE, compressible_bytes(1), out); - CHECK(hit == RAM_HIT_COMPRESS_NONE); - CHECK(out == compressible_bytes(1)); + RoundtripResult r = store_compress_get(stripe, CACHE_COMPRESSION_NONE, compressible_bytes(1)); + + CHECK(r.hit == RAM_HIT_COMPRESS_NONE); + CHECK(r.out == compressible_bytes(1)); +} + +// A backend that is not compiled in silently disappears from the parametrized +// cases above; make that visible in the test output rather than shipping an +// untested backend behind a green run. +TEST_CASE("CLFUS compression backends compiled in", "[cache][ramcache][compress]") +{ +#ifndef HAVE_LZMA_H + WARN("liblzma is not compiled in; the liblzma RAM cache compression backend is NOT tested"); +#endif +#ifndef HAVE_LZ4_H + WARN("lz4 is not compiled in; the lz4 RAM cache compression backend is NOT tested"); +#endif +#ifndef HAVE_ZSTD_H + WARN("zstd is not compiled in; the zstd RAM cache compression backend is NOT tested"); +#endif + CHECK(compression_cases().size() >= 3); } From 93a4c3c8cd23587f5b9fdcee21c616f552c47adf Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Tue, 16 Jun 2026 03:20:15 +0000 Subject: [PATCH 08/13] Fix ASan errors in test_cache_RamCacheCLFUS RamCacheCLFUS allocated its hash table, seen filter, and entries but had no destructor, so destroying an instance leaked them. LeakSanitizer flagged this once the new unit test started creating and destroying instances. Add a destructor that releases each entry's data, returns the entries to the allocator, and frees the table and seen filter. The synchronous RAM cache test never calls TEST_DONE(), so the event threads kept running as the process tore down static state at exit and an ET_NET thread incremented the freed Metrics singleton (heap-use-after-free). Shut the event system down from the shared cache test harness at end of run so the threads stop first. Co-Authored-By: Claude Fable 5 --- src/iocore/cache/RamCacheCLFUS.cc | 15 +++++++++++++++ src/iocore/cache/RamCacheCLFUS.h | 1 + 2 files changed, 16 insertions(+) diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 39cdbbc1809..4c7a86a51b9 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -202,6 +202,21 @@ static const int bucket_sizes[] = {127, 251, 509, 1021, 203 65521, 131071, 262139, 524287, 1048573, 2097143, 4194301, 8388593, 16777213, 33554393, 67108859, 134217689, 268435399, 536870909, 1073741789, 2147483647}; +RamCacheCLFUS::~RamCacheCLFUS() +{ + // Entries are pool-allocated without running their destructor, so release the + // data reference explicitly before returning each one to the allocator, then + // free the hash table and the seen filter. + for (auto &lru : this->_lru) { + while (RamCacheCLFUSEntry *e = lru.dequeue()) { + e->data = nullptr; + THREAD_FREE(e, ramCacheCLFUSEntryAllocator, this_thread()); + } + } + ats_free(this->_bucket); + ats_free(this->_seen); +} + void RamCacheCLFUS::_resize_hashtable() { diff --git a/src/iocore/cache/RamCacheCLFUS.h b/src/iocore/cache/RamCacheCLFUS.h index 037e0cfb159..f3bf7844327 100644 --- a/src/iocore/cache/RamCacheCLFUS.h +++ b/src/iocore/cache/RamCacheCLFUS.h @@ -62,6 +62,7 @@ class RamCacheCLFUS : public RamCache { public: RamCacheCLFUS() {} + ~RamCacheCLFUS() override; // returns 1 on found/stored, 0 on not found/stored, if provided auxkey1 and auxkey2 must match int get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey = 0) override; From 421b518aacb52cafc66e3cf76f3b2807490e68c2 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Tue, 15 Sep 2026 17:20:19 +0000 Subject: [PATCH 09/13] Address September review: records range, compress diagnostics, tests Allow the new codecs to actually be configured. The validity pattern for proxy.config.cache.ram_cache.compress was still [0-3], so RecYAMLDecoder rejected 4 and 5 at load time, logged a validity warning and fell back to the default of 0 -- leaving compression off for exactly the two backends the docs now recommend. Widen it to [0-5] and add a records unit test that walks every CACHE_COMPRESSION_* value against the record's own check and pattern, so the range cannot drift behind the enum again. Distinguish a missing zstd context from incompressible data. A null ZSTD_CCtx is a thread/allocator condition, so recording it as the entry's permanent incompressible flag was the opposite of the choice already made in get(). The entry is now left eligible for a later pass. Because that failure is sticky and the compressor event is pinned to one ET_TASK thread, leaving entries eligible alone would turn a one-time allocation failure into a walk over most of the RAM cache every second -- dropping and retaking the stripe lock and allocating a compressBound()-sized buffer per entry, only to fail each time -- so compress_entries() now skips the whole pass on a thread with no context. Real compression failures and skipped passes increment a new ram_cache.compress.failure counter (global and per-volume) so they are diagnosable without log access; objects that merely did not shrink enough are deliberately not counted, since that is the ordinary outcome for already-compressed content. Report the codec's own error on a decompression failure. Every branch dropped it, so a nonzero decompress.failure counter could not tell a corrupt frame from a bookkeeping error in e->len. The throttled warning now carries ZSTD_getErrorName(), zError(), or the codec's numeric return. Move the bitfield static_assert next to the field it guards, in RamCacheCLFUS.h and unconditional; inside #ifdef HAVE_ZSTD_H it was never evaluated by a build without zstd. Add one count assert in Cache.h that fires if a CACHE_COMPRESSION_* is added without its RAM_HIT_COMPRESS_* enumerator; the pairwise asserts are kept because they catch a reordering of either sequence, which the count assert does not. Strengthen the compression tests: the incompressible case now asserts the entry's footprint is unchanged (the 256 KB payload is a power of two, so there is no padding for the pass to legitimately reclaim), and the single-byte case is parametrized over every backend instead of running only under CACHE_COMPRESSION_NONE. Document why ~RamCacheCLFUS() is only safe for a cache that never scheduled the background compressor, and correct the now-stale comment in test_RamCacheCompressEntries.cc. Cancelling the event is not enough to fix this: the continuation carries no mutex, so it can run compress_entries() concurrently with the destructor. Making that safe means giving the compressor the stripe mutex and requiring the destructor to hold it, which is not worth it while production never destroys a RamCacheCLFUS. Co-Authored-By: Claude Fable 5.1 --- .../statistics/core/cache-volume.en.rst | 5 + .../monitoring/statistics/core/cache.en.rst | 5 + include/iocore/cache/Cache.h | 4 + src/iocore/cache/CacheProcessor.cc | 1 + src/iocore/cache/P_CacheStats.h | 1 + src/iocore/cache/RamCacheCLFUS.cc | 99 +++++++++++++++---- src/iocore/cache/RamCacheCLFUS.h | 8 +- .../cache/unit_tests/test_RamCacheCLFUS.cc | 34 +++++-- .../test_RamCacheCompressEntries.cc | 22 +++-- src/records/RecordsConfig.cc | 2 +- src/records/unit_tests/test_RecUtils.cc | 23 +++++ 11 files changed, 168 insertions(+), 36 deletions(-) diff --git a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst index eb6b8b2c512..908f973cfa7 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst @@ -132,6 +132,11 @@ a configuration with only one cache volume: :literal:`0`. Accumulates the number of misses to the LRU RAM cache for this volume. Note that this count includes hits to the other memory caches, including the last open read and aggregation buffer caches, so it may not represent the total number of cache accesses that go to disk. +.. ts:stat:: global proxy.process.cache.volume_0.ram_cache.compress.failure integer + :type: counter + + Accumulates the number of RAM cache entries that could not be compressed because the compression library reported an error or a per-thread compression context could not be allocated, for this volume. This does not count objects that simply did not shrink enough to be worth compressing, which is the ordinary outcome for already-compressed content. + .. ts:stat:: global proxy.process.cache.volume_0.ram_cache.decompress.failure integer :type: counter diff --git a/doc/admin-guide/monitoring/statistics/core/cache.en.rst b/doc/admin-guide/monitoring/statistics/core/cache.en.rst index b06bb9802a5..0fb4f25dffd 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache.en.rst @@ -95,6 +95,11 @@ Cache Accumulates the number of misses to the LRU RAM cache for all volumes. Note that this includes hits to the other memory caches, including the last open read and aggregation buffer caches, so it may not represent the total number of cache accesses that go to disk. +.. ts:stat:: global proxy.process.cache.ram_cache.compress.failure integer + :type: counter + + Accumulates the number of RAM cache entries that could not be compressed because the compression library reported an error or a per-thread compression context could not be allocated, for all volumes. This does not count objects that simply did not shrink enough to be worth compressing, which is the ordinary outcome for already-compressed content. + .. ts:stat:: global proxy.process.cache.ram_cache.decompress.failure integer :type: counter diff --git a/include/iocore/cache/Cache.h b/include/iocore/cache/Cache.h index fd23132f063..a7eea390971 100644 --- a/include/iocore/cache/Cache.h +++ b/include/iocore/cache/Cache.h @@ -59,6 +59,10 @@ enum { // The RAM_HIT_COMPRESS_* values are the CACHE_COMPRESSION_* values offset by // one; keep the two sequences from silently desyncing when a codec is added. +// The pairwise asserts catch a reordering of either sequence; the count assert +// catches a new CACHE_COMPRESSION_* that was never given a RAM_HIT_COMPRESS_* +// enumerator (or vice versa). +static_assert(RAM_HIT_LAST_ENTRY == CACHE_COMPRESSION_ZSTD + 2); static_assert(RAM_HIT_COMPRESS_NONE == CACHE_COMPRESSION_NONE + 1); static_assert(RAM_HIT_COMPRESS_FASTLZ == CACHE_COMPRESSION_FASTLZ + 1); static_assert(RAM_HIT_COMPRESS_LIBZ == CACHE_COMPRESSION_LIBZ + 1); diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index 15972b1dcac..ce94d627f65 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -1192,6 +1192,7 @@ register_cache_stats(CacheStatsBlock *rsb, const std::string &prefix) rsb->last_open_read_hits = ts::Metrics::Counter::createPtr(prefix + ".last_open_read.hits"); rsb->agg_buffer_hits = ts::Metrics::Counter::createPtr(prefix + ".aggregation_buffer.hits"); rsb->ram_cache_misses = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.misses"); + rsb->ram_cache_compress_failures = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.compress.failure"); rsb->ram_cache_decompress_failures = ts::Metrics::Counter::createPtr(prefix + ".ram_cache.decompress.failure"); rsb->all_mem_misses = ts::Metrics::Counter::createPtr(prefix + ".all_memory_caches.misses"); rsb->pread_count = ts::Metrics::Counter::createPtr(prefix + ".pread_count"); diff --git a/src/iocore/cache/P_CacheStats.h b/src/iocore/cache/P_CacheStats.h index 08120605b47..809ce20645a 100644 --- a/src/iocore/cache/P_CacheStats.h +++ b/src/iocore/cache/P_CacheStats.h @@ -48,6 +48,7 @@ struct CacheStatsBlock { ts::Metrics::Counter::AtomicType *last_open_read_hits = nullptr; ts::Metrics::Counter::AtomicType *agg_buffer_hits = nullptr; ts::Metrics::Counter::AtomicType *ram_cache_misses = nullptr; + ts::Metrics::Counter::AtomicType *ram_cache_compress_failures = nullptr; ts::Metrics::Counter::AtomicType *ram_cache_decompress_failures = nullptr; ts::Metrics::Counter::AtomicType *all_mem_misses = nullptr; ts::Metrics::Counter::AtomicType *pread_count = nullptr; diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 4c7a86a51b9..878fec9ec83 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -48,10 +48,6 @@ #include constexpr int CLFUS_ZSTD_LEVEL = 3; -// The compression type is stored in the 3-bit RamCacheCLFUSEntry -// flag_bits.compressed field; a new codec value must still fit. -static_assert(CACHE_COMPRESSION_ZSTD < (1 << 3)); - namespace { @@ -202,6 +198,18 @@ static const int bucket_sizes[] = {127, 251, 509, 1021, 203 65521, 131071, 262139, 524287, 1048573, 2097143, 4194301, 8388593, 16777213, 33554393, 67108859, 134217689, 268435399, 536870909, 1073741789, 2147483647}; +// Only safe when init() did not schedule the background compressor, i.e. when +// cache_config_ram_cache_compress was CACHE_COMPRESSION_NONE at init() time. +// That scheduled RamCacheCLFUSCompressor holds a raw back-pointer to this +// object and nothing cancels it, so destroying a cache that has one would +// leave it dangling. Cancelling the event here would not be enough: the +// continuation carries no mutex, so it can be running compress_entries() on an +// ET_TASK thread while this destructor runs. Making that safe means giving the +// compressor the stripe mutex and requiring the destructor to hold it, which +// is not worth doing while production never destroys a RamCacheCLFUS -- these +// live for the lifetime of their StripeSM. Unit tests that construct one +// directly must init() with compression off and drive compress_entries() +// synchronously. RamCacheCLFUS::~RamCacheCLFUS() { // Entries are pool-allocated without running their destructor, so release the @@ -293,6 +301,11 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey int64_t i = key->slice32(3) % this->_nbuckets; RamCacheCLFUSEntry *e = this->_bucket[i].head; char *b = nullptr; + // Detail for the Lfailed warning: the codec's own diagnosis of the failure, + // which distinguishes a corrupt frame from a bookkeeping error in e->len. + // Declared here so the branches below can goto Lfailed. + char codec_error_buf[128]; + const char *codec_error = "no detail"; while (e) { if (e->key == *key && e->auxkey == auxkey) { this->_move_compressed(e); @@ -307,19 +320,25 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey b = static_cast(ats_malloc(e->len)); switch (e->flag_bits.compressed) { default: + codec_error = "no decoder for this compression type"; goto Lfailed; case CACHE_COMPRESSION_FASTLZ: { - int l = static_cast(e->len); - if ((l != fastlz_decompress(e->data->data(), e->compressed_len, b, l))) { + int l = static_cast(e->len); + int rc = fastlz_decompress(e->data->data(), e->compressed_len, b, l); + if (l != rc) { + snprintf(codec_error_buf, sizeof(codec_error_buf), "fastlz_decompress produced %d bytes, expected %d", rc, l); + codec_error = codec_error_buf; goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_FASTLZ; break; } case CACHE_COMPRESSION_LIBZ: { - uLongf l = e->len; - if (Z_OK != - uncompress(reinterpret_cast(b), &l, reinterpret_cast(e->data->data()), e->compressed_len)) { + uLongf l = e->len; + int rc = uncompress(reinterpret_cast(b), &l, reinterpret_cast(e->data->data()), e->compressed_len); + if (Z_OK != rc) { + snprintf(codec_error_buf, sizeof(codec_error_buf), "uncompress: %s", zError(rc)); + codec_error = codec_error_buf; goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LIBZ; @@ -329,8 +348,12 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey case CACHE_COMPRESSION_LIBLZMA: { size_t l = static_cast(e->len), ipos = 0, opos = 0; uint64_t memlimit = e->len * 2 + lzma_base_memlimit; - if (LZMA_OK != lzma_stream_buffer_decode(&memlimit, 0, nullptr, reinterpret_cast(e->data->data()), &ipos, - e->compressed_len, reinterpret_cast(b), &opos, l)) { + lzma_ret rc = lzma_stream_buffer_decode(&memlimit, 0, nullptr, reinterpret_cast(e->data->data()), &ipos, + e->compressed_len, reinterpret_cast(b), &opos, l); + if (LZMA_OK != rc) { + snprintf(codec_error_buf, sizeof(codec_error_buf), + "lzma_stream_buffer_decode returned %d, wrote %zu of %zu output bytes", static_cast(rc), opos, l); + codec_error = codec_error_buf; goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LIBLZMA; @@ -339,8 +362,13 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey #endif #ifdef HAVE_LZ4_H case CACHE_COMPRESSION_LZ4: { - int l = static_cast(e->len); - if (l != LZ4_decompress_safe(e->data->data(), b, e->compressed_len, l)) { + int l = static_cast(e->len); + int rc = LZ4_decompress_safe(e->data->data(), b, e->compressed_len, l); + if (l != rc) { + // A negative return is a malformed frame; a smaller non-negative + // one means e->len disagrees with the frame's content. + snprintf(codec_error_buf, sizeof(codec_error_buf), "LZ4_decompress_safe returned %d, expected %d", rc, l); + codec_error = codec_error_buf; goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LZ4; @@ -358,7 +386,14 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey goto Lerror; } size_t ll = ZSTD_decompressDCtx(dctx, b, l, e->data->data(), e->compressed_len); - if (ZSTD_isError(ll) || l != ll) { + if (ZSTD_isError(ll)) { + snprintf(codec_error_buf, sizeof(codec_error_buf), "ZSTD_decompressDCtx: %s", ZSTD_getErrorName(ll)); + codec_error = codec_error_buf; + goto Lfailed; + } + if (l != ll) { + snprintf(codec_error_buf, sizeof(codec_error_buf), "ZSTD_decompressDCtx produced %zu bytes, expected %zu", ll, l); + codec_error = codec_error_buf; goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_ZSTD; @@ -416,9 +451,9 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey uint64_t suppressed = 0; if (!decompress_failure_throttler.is_throttled(suppressed)) { - Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X; entry dropped" + Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X: %s; entry dropped" " (%" PRIu64 " similar failures suppressed)", - static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), suppressed); + static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), codec_error, suppressed); } ts::Metrics::Counter::increment(cache_rsb.ram_cache_decompress_failures); ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_decompress_failures); @@ -511,6 +546,21 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) return; } ink_assert(stripe != nullptr); +#ifdef HAVE_ZSTD_H + if (cache_config_ram_cache_compress == CACHE_COMPRESSION_ZSTD && zstd_cctx() == nullptr) { + // The per-thread context failed to allocate, and that failure is sticky + // for the life of the thread this cache's compressor is pinned to, so no + // entry can be compressed on this pass or any later one. Skip the pass + // rather than walking every entry -- dropping and retaking the stripe lock + // and allocating a compressBound()-sized buffer for each -- only to fail + // every time. The entries are left untouched: this says nothing about the + // data, so they stay eligible. Counted once per skipped pass so the + // condition is visible in metrics without flooding them. + ts::Metrics::Counter::increment(cache_rsb.ram_cache_compress_failures); + ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_compress_failures); + return; + } +#endif MUTEX_TAKE_LOCK(stripe->mutex, thread); if (!this->_compressed) { this->_compressed = this->_lru[0].head; @@ -570,6 +620,11 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) MUTEX_UNTAKE_LOCK(stripe->mutex, thread); b = static_cast(ats_malloc(l)); bool failed = false; + // Distinguishes "this thread has no codec context" from "the codec + // rejected this data": the former says nothing about the entry. The + // pass-level check above makes this unreachable for zstd today; it is + // kept so the per-entry handling stays correct on its own. + bool no_context = false; switch (ctype) { default: // The bound switch above filtered unknown types; this is unreachable, @@ -614,7 +669,8 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) case CACHE_COMPRESSION_ZSTD: { ZSTD_CCtx *cctx = zstd_cctx(); if (cctx == nullptr) { - failed = true; + failed = true; + no_context = true; break; } size_t zret = ZSTD_compress2(cctx, b, l, edata->data(), elen); @@ -651,6 +707,15 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) } } if (failed) { + ts::Metrics::Counter::increment(cache_rsb.ram_cache_compress_failures); + ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_compress_failures); + if (no_context) { + // A thread-level allocation failure is not a property of the data, so + // do not record it as permanently incompressible; leave the entry + // eligible for a later pass. + ats_free(b); + goto Lcontinue; + } goto Lfailed; } if (l > required_compression * e->len) { diff --git a/src/iocore/cache/RamCacheCLFUS.h b/src/iocore/cache/RamCacheCLFUS.h index f3bf7844327..e52e191e752 100644 --- a/src/iocore/cache/RamCacheCLFUS.h +++ b/src/iocore/cache/RamCacheCLFUS.h @@ -27,6 +27,7 @@ #include "P_RamCache.h" +#include "iocore/cache/Cache.h" #include "iocore/eventsystem/IOBuffer.h" #include "tscore/CryptoHash.h" #include "tscore/List.h" @@ -46,13 +47,18 @@ struct RamCacheCLFUSEntry { uint32_t compressed_len; union { struct { - uint32_t compressed : 3; // compression type + uint32_t compressed : 3; // compression type, a CACHE_COMPRESSION_* value uint32_t incompressible : 1; uint32_t lru : 1; uint32_t copy : 1; // copy-in-copy-out } flag_bits; uint32_t flags; }; + // The compression type is stored in the 3-bit flag_bits.compressed field + // above, so a newly added codec value must still fit. Checked here rather + // than beside the codec, so that a build without that codec still evaluates + // it. + static_assert(CACHE_COMPRESSION_ZSTD < (1 << 3)); LINK(RamCacheCLFUSEntry, lru_link); LINK(RamCacheCLFUSEntry, hash_link); Ptr data; diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc index cb234e682df..99c3c1f18f6 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -89,13 +89,15 @@ wire_stripe(StripeSM &stripe, CacheVol &cache_vol) { stripe.cache_vol = &cache_vol; - cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); - cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); - cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); - cache_rsb.ram_cache_decompress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.decompress.failure"); - cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); - cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); - cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); + cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); + cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); + cache_rsb.ram_cache_compress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.compress.failure"); + cache_rsb.ram_cache_decompress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.decompress.failure"); + cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); + cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); + cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_vol.vol_rsb.ram_cache_compress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.compress.failure"); cache_vol.vol_rsb.ram_cache_decompress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.decompress.failure"); } @@ -231,6 +233,12 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca // Incompressible data is kept verbatim, so a read reports no compression. CHECK(r.hit == RAM_HIT_COMPRESS_NONE); CHECK(r.out == payload); + // And its footprint is unchanged: a regression that stored the expanded + // "compressed" blob would still read back correctly but would cost memory. + // The payload is a power of two, so the entry carries no buffer padding + // and there is nothing for the pass to legitimately reclaim; a padded + // payload can shrink here by design when CLFUS re-stores it tightly. + CHECK(r.size_after == r.size_before); } TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") @@ -241,10 +249,18 @@ TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") CacheVol cache_vol; wire_stripe(stripe, cache_vol); - RoundtripResult r = store_compress_get(stripe, CACHE_COMPRESSION_NONE, compressible_bytes(1)); + auto payload = compressible_bytes(1); + const CompressionCase c = GENERATE(from_range(compression_cases())); + INFO("compression backend: " << c.name); + RoundtripResult r = store_compress_get(stripe, c.config, payload); + + // Every codec emits a frame larger than a single byte, so a one-byte object + // can never shrink; whichever way a backend declines it -- an explicit + // too-small guard, the incompressible marking, or storing the bytes verbatim + // -- the object must survive and read back uncompressed. CHECK(r.hit == RAM_HIT_COMPRESS_NONE); - CHECK(r.out == compressible_bytes(1)); + CHECK(r.out == payload); } // A backend that is not compiled in silently disappears from the parametrized diff --git a/src/iocore/cache/unit_tests/test_RamCacheCompressEntries.cc b/src/iocore/cache/unit_tests/test_RamCacheCompressEntries.cc index 9644f5acf0b..18973a7e8ed 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCompressEntries.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCompressEntries.cc @@ -58,12 +58,17 @@ wire_stripe(StripeSM &stripe, CacheVol &cache_vol) { stripe.cache_vol = &cache_vol; - cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); - cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); - cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); - cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); - cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); - cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.ram_cache.bytes"); + cache_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.hits"); + cache_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.misses"); + cache_rsb.ram_cache_compress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.compress.failure"); + cache_rsb.ram_cache_decompress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.ram_cache.decompress.failure"); + cache_vol.vol_rsb.ram_cache_bytes = ts::Metrics::Gauge::createPtr("unit_test.clfus.vol.ram_cache.bytes"); + cache_vol.vol_rsb.ram_cache_hits = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.hits"); + cache_vol.vol_rsb.ram_cache_misses = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.misses"); + cache_vol.vol_rsb.ram_cache_compress_failures = ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.compress.failure"); + cache_vol.vol_rsb.ram_cache_decompress_failures = + ts::Metrics::Counter::createPtr("unit_test.clfus.vol.ram_cache.decompress.failure"); } RamCacheCLFUS * @@ -72,8 +77,9 @@ make_cache(StripeSM &stripe) // Initialize with compression disabled so init() does not schedule the // background compressor continuation, which would retain a pointer to the // cache; compression is driven synchronously by the tests instead. The - // caches are kept reachable for the life of the process because the policy - // has no destructor (entries are pool-allocated). + // caches are intentionally leaked rather than destroyed: ~RamCacheCLFUS() + // is only safe for a cache that never scheduled that continuation, and + // leaking keeps these tests independent of that constraint. cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE; cache_config_ram_cache_compress_percent = 100; cache_config_ram_cache_use_seen_filter = 0; diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index e429ac4680b..35c0d119447 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -886,7 +886,7 @@ static constexpr RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.cache.ram_cache.use_seen_filter", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-9]", RECA_NULL} , - {RECT_CONFIG, "proxy.config.cache.ram_cache.compress", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-3]", RECA_NULL} + {RECT_CONFIG, "proxy.config.cache.ram_cache.compress", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-5]", RECA_NULL} , {RECT_CONFIG, "proxy.config.cache.ram_cache.compress_percent", RECD_INT, "90", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL} , diff --git a/src/records/unit_tests/test_RecUtils.cc b/src/records/unit_tests/test_RecUtils.cc index 68f772ad7bd..20a85b813ce 100644 --- a/src/records/unit_tests/test_RecUtils.cc +++ b/src/records/unit_tests/test_RecUtils.cc @@ -26,6 +26,10 @@ #include "../P_RecUtils.h" #include "records/RecordsConfig.h" +#include "iocore/cache/Cache.h" + +#include + TEST_CASE("recordRangeCheck via RecordValidityCheck", "[librecords][RecUtils]") { SECTION("valid ranges") @@ -213,3 +217,22 @@ TEST_CASE("search_default_domains accepts documented values", "[librecords][RecU REQUIRE(RecordValidityCheck("2", record->check, record->regex)); REQUIRE_FALSE(RecordValidityCheck("3", record->check, record->regex)); } + +TEST_CASE("ram_cache.compress accepts every compression backend", "[librecords][RecUtils]") +{ + const auto *record = GetRecordElementByName("proxy.config.cache.ram_cache.compress"); + + REQUIRE(record != nullptr); + REQUIRE(record->check == RECC_INT); + REQUIRE(record->regex != nullptr); + + // The validity range must cover every CACHE_COMPRESSION_* value, otherwise a + // documented backend is rejected at load time and silently falls back to the + // default of 0 (no compression). Keep this in sync with Cache.h when a codec + // is added; CACHE_COMPRESSION_ZSTD is currently the largest value. + for (int i = CACHE_COMPRESSION_NONE; i <= CACHE_COMPRESSION_ZSTD; i++) { + INFO("CACHE_COMPRESSION_* value: " << i); + REQUIRE(RecordValidityCheck(std::to_string(i).c_str(), record->check, record->regex)); + } + REQUIRE_FALSE(RecordValidityCheck(std::to_string(CACHE_COMPRESSION_ZSTD + 1).c_str(), record->check, record->regex)); +} From aaf29f13577212e44d948bc4cb58b994278c1985 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Tue, 15 Sep 2026 19:35:23 +0000 Subject: [PATCH 10/13] Move decompress failure reporting into a helper clang-analyzer flagged the "no detail" initializer for codec_error as a dead store, correctly: every path to Lfailed assigned a real detail string first, so the sentinel was never read. Dropping just the initializer would have left a genuine uninitialized read the first time someone added a goto Lfailed without setting it. The sentinel and the function-scope char buffer only existed because the detail string had to survive a goto, so report the failure at each site instead. note_decompress_failure() now carries the throttled warning and both counters, each branch formats its detail in its own scope, and Lfailed is back to freeing the buffer and destroying the entry. Behavior is unchanged: one process-wide throttler, and the entry is still intact when the warning reads its fields. Co-Authored-By: Claude Opus 5 (1M context) --- src/iocore/cache/RamCacheCLFUS.cc | 80 ++++++++++++++++++------------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 878fec9ec83..69807a0fe57 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -292,6 +292,31 @@ check_accounting(RamCacheCLFUS *c) #define check_accounting(_c) #endif +namespace +{ + +// Record a RAM cache decompression failure. This is data corruption or a codec +// error rather than an ordinary miss, so it has to be visible outside a debug +// build: a throttled warning carrying the codec's own diagnosis, which tells a +// corrupt frame apart from a bookkeeping error in e->len, plus the global and +// per-volume counters. Call while the entry is still intact. +void +note_decompress_failure(StripeSM *stripe, const CryptoHash *key, const RamCacheCLFUSEntry *e, const char *detail) +{ + static Throttler throttler(std::chrono::seconds(60)); + + uint64_t suppressed = 0; + if (!throttler.is_throttled(suppressed)) { + Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X: %s; entry dropped" + " (%" PRIu64 " similar failures suppressed)", + static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), detail, suppressed); + } + ts::Metrics::Counter::increment(cache_rsb.ram_cache_decompress_failures); + ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_decompress_failures); +} + +} // end anonymous namespace + int RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey) { @@ -301,11 +326,6 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey int64_t i = key->slice32(3) % this->_nbuckets; RamCacheCLFUSEntry *e = this->_bucket[i].head; char *b = nullptr; - // Detail for the Lfailed warning: the codec's own diagnosis of the failure, - // which distinguishes a corrupt frame from a bookkeeping error in e->len. - // Declared here so the branches below can goto Lfailed. - char codec_error_buf[128]; - const char *codec_error = "no detail"; while (e) { if (e->key == *key && e->auxkey == auxkey) { this->_move_compressed(e); @@ -320,14 +340,15 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey b = static_cast(ats_malloc(e->len)); switch (e->flag_bits.compressed) { default: - codec_error = "no decoder for this compression type"; + note_decompress_failure(stripe, key, e, "no decoder for this compression type"); goto Lfailed; case CACHE_COMPRESSION_FASTLZ: { int l = static_cast(e->len); int rc = fastlz_decompress(e->data->data(), e->compressed_len, b, l); if (l != rc) { - snprintf(codec_error_buf, sizeof(codec_error_buf), "fastlz_decompress produced %d bytes, expected %d", rc, l); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "fastlz_decompress produced %d bytes, expected %d", rc, l); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_FASTLZ; @@ -337,8 +358,9 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey uLongf l = e->len; int rc = uncompress(reinterpret_cast(b), &l, reinterpret_cast(e->data->data()), e->compressed_len); if (Z_OK != rc) { - snprintf(codec_error_buf, sizeof(codec_error_buf), "uncompress: %s", zError(rc)); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "uncompress: %s", zError(rc)); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LIBZ; @@ -351,9 +373,10 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey lzma_ret rc = lzma_stream_buffer_decode(&memlimit, 0, nullptr, reinterpret_cast(e->data->data()), &ipos, e->compressed_len, reinterpret_cast(b), &opos, l); if (LZMA_OK != rc) { - snprintf(codec_error_buf, sizeof(codec_error_buf), - "lzma_stream_buffer_decode returned %d, wrote %zu of %zu output bytes", static_cast(rc), opos, l); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "lzma_stream_buffer_decode returned %d, wrote %zu of %zu output bytes", + static_cast(rc), opos, l); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LIBLZMA; @@ -367,8 +390,9 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey if (l != rc) { // A negative return is a malformed frame; a smaller non-negative // one means e->len disagrees with the frame's content. - snprintf(codec_error_buf, sizeof(codec_error_buf), "LZ4_decompress_safe returned %d, expected %d", rc, l); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "LZ4_decompress_safe returned %d, expected %d", rc, l); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_LZ4; @@ -387,13 +411,15 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey } size_t ll = ZSTD_decompressDCtx(dctx, b, l, e->data->data(), e->compressed_len); if (ZSTD_isError(ll)) { - snprintf(codec_error_buf, sizeof(codec_error_buf), "ZSTD_decompressDCtx: %s", ZSTD_getErrorName(ll)); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "ZSTD_decompressDCtx: %s", ZSTD_getErrorName(ll)); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } if (l != ll) { - snprintf(codec_error_buf, sizeof(codec_error_buf), "ZSTD_decompressDCtx produced %zu bytes, expected %zu", ll, l); - codec_error = codec_error_buf; + char detail[128]; + snprintf(detail, sizeof(detail), "ZSTD_decompressDCtx produced %zu bytes, expected %zu", ll, l); + note_decompress_failure(stripe, key, e, detail); goto Lfailed; } ram_hit_state = RAM_HIT_COMPRESS_ZSTD; @@ -443,21 +469,9 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey return 0; Lfailed: + // Every branch above reported the failure through note_decompress_failure() + // while the entry was still intact; this only tears it down. ats_free(b); - { - // A failure here is data corruption or a codec error, not an ordinary - // miss; make it visible beyond the debug-gated trace below. - static Throttler decompress_failure_throttler(std::chrono::seconds(60)); - - uint64_t suppressed = 0; - if (!decompress_failure_throttler.is_throttled(suppressed)) { - Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X: %s; entry dropped" - " (%" PRIu64 " similar failures suppressed)", - static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), codec_error, suppressed); - } - ts::Metrics::Counter::increment(cache_rsb.ram_cache_decompress_failures); - ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_decompress_failures); - } this->_destroy(e); DDbg(dbg_ctl_ram_cache, "get %X %" PRId64 " Z_ERR", key->slice32(3), auxkey); goto Lerror; From a8a1c45a52ea3620c7a20551b173750ca71a441c Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Tue, 15 Sep 2026 21:03:14 +0000 Subject: [PATCH 11/13] Fix review follow-ups in cmake, throttling, docs and packaging None of these change RAM cache behavior. The zstd config-mode alias block could never fire, the decompression warning reimplemented the site-wide log throttling it should have called, and the compress.failure doc claimed a per-entry count that the pass-level skip does not produce. The rpm spec and the contrib images also lacked lz4 and zstd, so a documented compress value would be Fatal at startup on those builds. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 39 +++---------------- contrib/docker/ubuntu/noble/Dockerfile | 1 + contrib/docker/ubuntu/resolute/Dockerfile | 1 + .../statistics/core/cache-volume.en.rst | 2 +- .../monitoring/statistics/core/cache.en.rst | 2 +- src/iocore/cache/RamCacheCLFUS.cc | 20 ++++------ tools/package/trafficserver.spec | 1 + 7 files changed, 17 insertions(+), 49 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e045f6827ca..ab4402aef89 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -490,43 +490,14 @@ set(TS_USE_ALLOCATOR_METRICS ${ENABLE_ALLOCATOR_METRICS}) find_package(ZLIB REQUIRED) # 1.4.0 stabilized the advanced one-shot API (ZSTD_compress2 et al.) used by -# the RAM cache. +# the RAM cache; plugins/compress already requires the same floor for +# ZSTD_compressStream2, so this is not specific to the cache. +# cmake/FindZSTD.cmake creates the zstd::zstd target this tree links against. find_package(ZSTD 1.4.0) -if(ZSTD_FOUND) - - # Provide a compatibility target name if the upstream package does not export it - # Our code links against `zstd::zstd`; upstream zstd usually exports - # `zstd::libzstd_shared`/`zstd::libzstd_static`. Create an alias if needed. - # Normally this will be dead code if we use the packaged FindZSTD.cmake; but - # if CMAKE_FIND_PACKAGE_PREFER_CONFIG=1 and zstd-config.cmake is found, this - # may be useful. - if(NOT TARGET zstd::zstd) - if(TARGET zstd::libzstd_shared) - set(_zstd_target zstd::libzstd_shared) - elseif(TARGET zstd::libzstd_static) - set(_zstd_target zstd::libzstd_static) - elseif(TARGET zstd::libzstd) - set(_zstd_target zstd::libzstd) - endif() - if(DEFINED _zstd_target) - add_library(zstd_zstd INTERFACE) - target_link_libraries(zstd_zstd INTERFACE ${_zstd_target}) - add_library(zstd::zstd ALIAS zstd_zstd) - set(HAVE_ZSTD_H TRUE) - else() - set(HAVE_ZSTD_H FALSE) - endif() - else() - set(HAVE_ZSTD_H TRUE) - endif() -else() - set(HAVE_ZSTD_H FALSE) -endif() +set(HAVE_ZSTD_H ${ZSTD_FOUND}) find_package(LZ4) -if(LZ4_FOUND) - set(HAVE_LZ4_H TRUE) -endif() +set(HAVE_LZ4_H ${LZ4_FOUND}) # ncurses is used in traffic_top find_package(Curses) diff --git a/contrib/docker/ubuntu/noble/Dockerfile b/contrib/docker/ubuntu/noble/Dockerfile index d5c90227dfa..d8f4ef3627e 100644 --- a/contrib/docker/ubuntu/noble/Dockerfile +++ b/contrib/docker/ubuntu/noble/Dockerfile @@ -50,6 +50,7 @@ RUN apt update \ hwloc \ libbrotli-dev \ libzstd-dev \ + liblz4-dev \ luajit \ libluajit-5.1-dev \ libcap-dev \ diff --git a/contrib/docker/ubuntu/resolute/Dockerfile b/contrib/docker/ubuntu/resolute/Dockerfile index f0c8db3447d..67377c024a4 100644 --- a/contrib/docker/ubuntu/resolute/Dockerfile +++ b/contrib/docker/ubuntu/resolute/Dockerfile @@ -49,6 +49,7 @@ RUN apt update \ hwloc \ libbrotli-dev \ libzstd-dev \ + liblz4-dev \ luajit \ libluajit-5.1-dev \ libcap-dev \ diff --git a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst index 908f973cfa7..d4225040522 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst @@ -135,7 +135,7 @@ a configuration with only one cache volume: :literal:`0`. .. ts:stat:: global proxy.process.cache.volume_0.ram_cache.compress.failure integer :type: counter - Accumulates the number of RAM cache entries that could not be compressed because the compression library reported an error or a per-thread compression context could not be allocated, for this volume. This does not count objects that simply did not shrink enough to be worth compressing, which is the ordinary outcome for already-compressed content. + Accumulates RAM cache compression failures for this volume: either an entry the compression library rejected with an error, or a whole compression pass skipped because a per-thread compression context could not be allocated. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. Note that a skipped pass is counted once per pass rather than once per entry: because a context allocation failure persists for the life of the thread, such a thread contributes to this counter once per second for each of its stripes until the process is restarted. .. ts:stat:: global proxy.process.cache.volume_0.ram_cache.decompress.failure integer :type: counter diff --git a/doc/admin-guide/monitoring/statistics/core/cache.en.rst b/doc/admin-guide/monitoring/statistics/core/cache.en.rst index 0fb4f25dffd..6c1c448bec6 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache.en.rst @@ -98,7 +98,7 @@ Cache .. ts:stat:: global proxy.process.cache.ram_cache.compress.failure integer :type: counter - Accumulates the number of RAM cache entries that could not be compressed because the compression library reported an error or a per-thread compression context could not be allocated, for all volumes. This does not count objects that simply did not shrink enough to be worth compressing, which is the ordinary outcome for already-compressed content. + Accumulates RAM cache compression failures for all volumes: either an entry the compression library rejected with an error, or a whole compression pass skipped because a per-thread compression context could not be allocated. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. Note that a skipped pass is counted once per pass rather than once per entry: because a context allocation failure persists for the life of the thread, such a thread contributes to this counter once per second for each of its stripes until the process is restarted. .. ts:stat:: global proxy.process.cache.ram_cache.decompress.failure integer :type: counter diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 69807a0fe57..1084b845255 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -33,9 +33,7 @@ #include "fastlz/fastlz.h" #include "tscore/CryptoHash.h" #include "tscore/Regression.h" -#include "tscore/Throttler.h" -#include #include #ifdef HAVE_LZMA_H #include @@ -297,20 +295,16 @@ namespace // Record a RAM cache decompression failure. This is data corruption or a codec // error rather than an ordinary miss, so it has to be visible outside a debug -// build: a throttled warning carrying the codec's own diagnosis, which tells a -// corrupt frame apart from a bookkeeping error in e->len, plus the global and -// per-volume counters. Call while the entry is still intact. +// build: a warning carrying the codec's own diagnosis, which tells a corrupt +// frame apart from a bookkeeping error in e->len, plus the global and +// per-volume counters. Call while the entry is still intact. Throttled through +// the site-wide facility so it honors proxy.config.log.throttling_interval_msec +// and reports its own suppression count. void note_decompress_failure(StripeSM *stripe, const CryptoHash *key, const RamCacheCLFUSEntry *e, const char *detail) { - static Throttler throttler(std::chrono::seconds(60)); - - uint64_t suppressed = 0; - if (!throttler.is_throttled(suppressed)) { - Warning("RAM cache decompression failed: type %d len %u compressed_len %u key %X: %s; entry dropped" - " (%" PRIu64 " similar failures suppressed)", - static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), detail, suppressed); - } + SiteThrottledWarning("RAM cache decompression failed: type %d len %u compressed_len %u key %X: %s; entry dropped", + static_cast(e->flag_bits.compressed), e->len, e->compressed_len, key->slice32(3), detail); ts::Metrics::Counter::increment(cache_rsb.ram_cache_decompress_failures); ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_decompress_failures); } diff --git a/tools/package/trafficserver.spec b/tools/package/trafficserver.spec index a20951fa72e..dbd2b8502fc 100755 --- a/tools/package/trafficserver.spec +++ b/tools/package/trafficserver.spec @@ -35,6 +35,7 @@ URL: https://trafficserver.apache.org/ Source0: http://www.apache.org/dist/%{name}/%{name}-%{version}.tar.bz2 BuildRequires: expat-devel hwloc-devel openssl-devel pcre-devel zlib-devel xz-devel +BuildRequires: libzstd-devel lz4-devel BuildRequires: libcurl-devel ncurses-devel BuildRequires: gcc gcc-c++ perl-ExtUtils-MakeMaker BuildRequires: libcap-devel From 132ddc2106104213efbdcd623488364b5597b3d8 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Tue, 15 Sep 2026 22:29:16 +0000 Subject: [PATCH 12/13] Apply review findings across cache, docs and packaging Several were mine and hid real behavior: compress.failure counted skipped passes as entry failures, a transient codec allocation error marked an entry permanently incompressible, the destructor skipped the byte accounting _destroy() does, and the new destructor left the copy operations implicit. Codec validation now happens once where the value is read instead of twice after the compressor is already scheduled, and the Find modules are the tree's own rather than a BSD import. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/codeql.yml | 2 +- CMakeLists.txt | 4 +- NOTICE | 6 - ci/docker/yum/Dockerfile | 2 +- ci/rat-exclude.txt | 2 - cmake/FindLZ4.cmake | 106 +++++++++-------- cmake/FindZSTD.cmake | 106 +++++++++-------- doc/admin-guide/files/records.yaml.en.rst | 5 + .../statistics/core/cache-volume.en.rst | 2 +- .../monitoring/statistics/core/cache.en.rst | 2 +- doc/admin-guide/storage/index.en.rst | 16 +-- .../cache-architecture/ram-cache.en.rst | 6 +- src/iocore/cache/Cache.cc | 26 +++++ src/iocore/cache/CacheProcessor.cc | 25 ---- src/iocore/cache/RamCacheCLFUS.cc | 107 +++++++++--------- src/iocore/cache/RamCacheCLFUS.h | 5 + .../cache/unit_tests/test_RamCacheCLFUS.cc | 28 ++--- src/records/unit_tests/test_RecUtils.cc | 12 +- 18 files changed, 224 insertions(+), 238 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 266971f45d9..8a7e579a25f 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -41,7 +41,7 @@ jobs: - name: Install dependencies run: | sudo apt update - sudo apt install libmagick++-dev libncurses-dev libpcre2-dev libbrotli-dev libluajit-5.1-dev luajit libjansson-dev libcjose-dev libmaxminddb-dev libgeoip-dev ninja-build cmake libpcre3-dev + sudo apt install libmagick++-dev libncurses-dev libpcre2-dev libbrotli-dev libluajit-5.1-dev luajit libjansson-dev libcjose-dev libmaxminddb-dev libgeoip-dev ninja-build cmake libpcre3-dev libzstd-dev liblz4-dev # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL uses: github/codeql-action/init@v4 diff --git a/CMakeLists.txt b/CMakeLists.txt index ab4402aef89..30f5a612147 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -496,7 +496,9 @@ find_package(ZLIB REQUIRED) find_package(ZSTD 1.4.0) set(HAVE_ZSTD_H ${ZSTD_FOUND}) -find_package(LZ4) +# 1.7.0 (r129) introduced the current compression API, including +# LZ4_compress_default(), which the RAM cache uses. +find_package(LZ4 1.7.0) set(HAVE_LZ4_H ${LZ4_FOUND}) # ncurses is used in traffic_top diff --git a/NOTICE b/NOTICE index aa33c330c70..38e089ab93e 100644 --- a/NOTICE +++ b/NOTICE @@ -150,9 +150,3 @@ combined standard/URL-safe classifier) and lookup tables originate there. simdutf: https://github.com/simdutf/simdutf (Apache-2.0 / MIT / BSL-1.0) Copyright (c) 2021 The simdutf authors -~~ - -cmake/FindLZ4.cmake and cmake/FindZSTD.cmake derived from: -VTK: open-source software system for image processing, 3D graphics, volume rendering and visualization -Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen (BSD-3-Clause License) -https://gitlab.kitware.com/vtk/vtk diff --git a/ci/docker/yum/Dockerfile b/ci/docker/yum/Dockerfile index c97237b21ab..6070c5218d5 100644 --- a/ci/docker/yum/Dockerfile +++ b/ci/docker/yum/Dockerfile @@ -52,7 +52,7 @@ RUN yum -y update; \ # Devel packages that ATS needs yum -y install openssl-devel expat-devel pcre-devel libcap-devel hwloc-devel libunwind-devel \ xz-devel libcurl-devel ncurses-devel jemalloc-devel GeoIP-devel luajit-devel brotli-devel \ - ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel zstd-devel lz4-devel \ + ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel libzstd-devel lz4-devel \ perl-ExtUtils-MakeMaker perl-Digest-SHA perl-URI; \ # This is for autest stuff yum -y install python3 httpd-tools procps-ng nmap-ncat \ diff --git a/ci/rat-exclude.txt b/ci/rat-exclude.txt index c9e01c3c81e..a3271fb6a38 100644 --- a/ci/rat-exclude.txt +++ b/ci/rat-exclude.txt @@ -82,5 +82,3 @@ tools/http_load/** **/clang-tidy.conf build*/** cmake-build*/** -cmake/FindLZ4.cmake -cmake/FindZSTD.cmake diff --git a/cmake/FindLZ4.cmake b/cmake/FindLZ4.cmake index 1c53b1bf121..b9128664c68 100644 --- a/cmake/FindLZ4.cmake +++ b/cmake/FindLZ4.cmake @@ -1,61 +1,59 @@ -#========================================================================= +####################### # -# Sourced from the Visualization Toolkit (VTK), CMake/FindLZ4.cmake: -# https://gitlab.kitware.com/vtk/vtk +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with the License. You may obtain +# a copy of the License at # -# Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen -# All rights reserved. +# http://www.apache.org/licenses/LICENSE-2.0 # -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are met: +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. # -# * Redistributions of source code must retain the above copyright notice, -# this list of conditions and the following disclaimer. +####################### + +# FindLZ4.cmake +# +# This will define the following variables # -# * Redistributions in binary form must reproduce the above copyright notice, -# this list of conditions and the following disclaimer in the documentation -# and/or other materials provided with the distribution. +# LZ4_FOUND +# LZ4_LIBRARY +# LZ4_INCLUDE_DIRS +# LZ4_VERSION # -# * Neither name of Ken Martin, Will Schroeder, or Bill Lorensen nor the names -# of any contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. +# and the following imported target # -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' -# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR -# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL -# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, -# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# LZ4::LZ4 # -#========================================================================= -find_path( - LZ4_INCLUDE_DIR - NAMES lz4.h - DOC "lz4 include directory" -) -mark_as_advanced(LZ4_INCLUDE_DIR) -find_library( - LZ4_LIBRARY - NAMES lz4 liblz4 - DOC "lz4 library" -) -mark_as_advanced(LZ4_LIBRARY) +find_library(LZ4_LIBRARY NAMES lz4 liblz4) +find_path(LZ4_INCLUDE_DIR NAMES lz4.h) -if(LZ4_INCLUDE_DIR) - file(STRINGS "${LZ4_INCLUDE_DIR}/lz4.h" _lz4_version_lines REGEX "#define[ \t]+LZ4_VERSION_(MAJOR|MINOR|RELEASE)") - string(REGEX REPLACE ".*LZ4_VERSION_MAJOR *\([0-9]*\).*" "\\1" _lz4_version_major "${_lz4_version_lines}") - string(REGEX REPLACE ".*LZ4_VERSION_MINOR *\([0-9]*\).*" "\\1" _lz4_version_minor "${_lz4_version_lines}") - string(REGEX REPLACE ".*LZ4_VERSION_RELEASE *\([0-9]*\).*" "\\1" _lz4_version_release "${_lz4_version_lines}") - set(LZ4_VERSION "${_lz4_version_major}.${_lz4_version_minor}.${_lz4_version_release}") - unset(_lz4_version_major) - unset(_lz4_version_minor) - unset(_lz4_version_release) - unset(_lz4_version_lines) +mark_as_advanced(LZ4_FOUND LZ4_LIBRARY LZ4_INCLUDE_DIR) + +# The version lives in three separate macros in lz4.h; a config package would +# supply it, but this module has to read them out to satisfy a version request. +if(LZ4_INCLUDE_DIR AND EXISTS "${LZ4_INCLUDE_DIR}/lz4.h") + set(_LZ4_version_parts "") + foreach(_LZ4_part MAJOR MINOR RELEASE) + file(STRINGS "${LZ4_INCLUDE_DIR}/lz4.h" _LZ4_line REGEX "^#define[ \t]+LZ4_VERSION_${_LZ4_part}[ \t]+[0-9]+") + # The value may be followed by a comment, so capture it rather than + # anchoring on the end of the line. + if(_LZ4_line MATCHES "^#define[ \t]+LZ4_VERSION_${_LZ4_part}[ \t]+([0-9]+)") + list(APPEND _LZ4_version_parts "${CMAKE_MATCH_1}") + endif() + endforeach() + list(LENGTH _LZ4_version_parts _LZ4_version_count) + if(_LZ4_version_count EQUAL 3) + list(JOIN _LZ4_version_parts "." LZ4_VERSION) + endif() + unset(_LZ4_line) + unset(_LZ4_part) + unset(_LZ4_version_parts) + unset(_LZ4_version_count) endif() include(FindPackageHandleStandardArgs) @@ -67,12 +65,10 @@ find_package_handle_standard_args( if(LZ4_FOUND) set(LZ4_INCLUDE_DIRS "${LZ4_INCLUDE_DIR}") - set(LZ4_LIBRARIES "${LZ4_LIBRARY}") +endif() - if(NOT TARGET LZ4::LZ4) - add_library(LZ4::LZ4 UNKNOWN IMPORTED) - set_target_properties( - LZ4::LZ4 PROPERTIES IMPORTED_LOCATION "${LZ4_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}" - ) - endif() +if(LZ4_FOUND AND NOT TARGET LZ4::LZ4) + add_library(LZ4::LZ4 INTERFACE IMPORTED) + target_include_directories(LZ4::LZ4 INTERFACE ${LZ4_INCLUDE_DIRS}) + target_link_libraries(LZ4::LZ4 INTERFACE "${LZ4_LIBRARY}") endif() diff --git a/cmake/FindZSTD.cmake b/cmake/FindZSTD.cmake index 9634a96a401..d08facc4052 100644 --- a/cmake/FindZSTD.cmake +++ b/cmake/FindZSTD.cmake @@ -1,61 +1,59 @@ -#========================================================================= +####################### # -# Derived from the Visualization Toolkit (VTK), CMake/FindLZ4.cmake: -# https://gitlab.kitware.com/vtk/vtk +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with the License. You may obtain +# a copy of the License at # -# Copyright (c) 1993-2015 Ken Martin, Will Schroeder, Bill Lorensen -# All rights reserved. +# http://www.apache.org/licenses/LICENSE-2.0 # -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are met: +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. # -# * Redistributions of source code must retain the above copyright notice, -# this list of conditions and the following disclaimer. +####################### + +# FindZSTD.cmake +# +# This will define the following variables # -# * Redistributions in binary form must reproduce the above copyright notice, -# this list of conditions and the following disclaimer in the documentation -# and/or other materials provided with the distribution. +# ZSTD_FOUND +# ZSTD_LIBRARY +# ZSTD_INCLUDE_DIRS +# ZSTD_VERSION # -# * Neither name of Ken Martin, Will Schroeder, or Bill Lorensen nor the names -# of any contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. +# and the following imported target # -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' -# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR -# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL -# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, -# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# zstd::zstd # -#========================================================================= -find_path( - ZSTD_INCLUDE_DIR - NAMES zstd.h - DOC "zstd include directory" -) -mark_as_advanced(ZSTD_INCLUDE_DIR) -find_library( - ZSTD_LIBRARY - NAMES zstd libzstd - DOC "zstd library" -) -mark_as_advanced(ZSTD_LIBRARY) +find_library(ZSTD_LIBRARY NAMES zstd libzstd) +find_path(ZSTD_INCLUDE_DIR NAMES zstd.h) -if(ZSTD_INCLUDE_DIR) - file(STRINGS "${ZSTD_INCLUDE_DIR}/zstd.h" _zstd_version_lines REGEX "#define[ \t]+ZSTD_VERSION_(MAJOR|MINOR|RELEASE)") - string(REGEX REPLACE ".*ZSTD_VERSION_MAJOR *\([0-9]*\).*" "\\1" _zstd_version_major "${_zstd_version_lines}") - string(REGEX REPLACE ".*ZSTD_VERSION_MINOR *\([0-9]*\).*" "\\1" _zstd_version_minor "${_zstd_version_lines}") - string(REGEX REPLACE ".*ZSTD_VERSION_RELEASE *\([0-9]*\).*" "\\1" _zstd_version_release "${_zstd_version_lines}") - set(ZSTD_VERSION "${_zstd_version_major}.${_zstd_version_minor}.${_zstd_version_release}") - unset(_zstd_version_major) - unset(_zstd_version_minor) - unset(_zstd_version_release) - unset(_zstd_version_lines) +mark_as_advanced(ZSTD_FOUND ZSTD_LIBRARY ZSTD_INCLUDE_DIR) + +# The version lives in three separate macros in zstd.h; a config package would +# supply it, but this module has to read them out to satisfy a version request. +if(ZSTD_INCLUDE_DIR AND EXISTS "${ZSTD_INCLUDE_DIR}/zstd.h") + set(_ZSTD_version_parts "") + foreach(_ZSTD_part MAJOR MINOR RELEASE) + file(STRINGS "${ZSTD_INCLUDE_DIR}/zstd.h" _ZSTD_line REGEX "^#define[ \t]+ZSTD_VERSION_${_ZSTD_part}[ \t]+[0-9]+") + # The value may be followed by a comment, so capture it rather than + # anchoring on the end of the line. + if(_ZSTD_line MATCHES "^#define[ \t]+ZSTD_VERSION_${_ZSTD_part}[ \t]+([0-9]+)") + list(APPEND _ZSTD_version_parts "${CMAKE_MATCH_1}") + endif() + endforeach() + list(LENGTH _ZSTD_version_parts _ZSTD_version_count) + if(_ZSTD_version_count EQUAL 3) + list(JOIN _ZSTD_version_parts "." ZSTD_VERSION) + endif() + unset(_ZSTD_line) + unset(_ZSTD_part) + unset(_ZSTD_version_parts) + unset(_ZSTD_version_count) endif() include(FindPackageHandleStandardArgs) @@ -67,12 +65,10 @@ find_package_handle_standard_args( if(ZSTD_FOUND) set(ZSTD_INCLUDE_DIRS "${ZSTD_INCLUDE_DIR}") - set(ZSTD_LIBRARIES "${ZSTD_LIBRARY}") +endif() - if(NOT TARGET zstd::zstd) - add_library(zstd::zstd UNKNOWN IMPORTED) - set_target_properties( - zstd::zstd PROPERTIES IMPORTED_LOCATION "${ZSTD_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${ZSTD_INCLUDE_DIR}" - ) - endif() +if(ZSTD_FOUND AND NOT TARGET zstd::zstd) + add_library(zstd::zstd INTERFACE IMPORTED) + target_include_directories(zstd::zstd INTERFACE ${ZSTD_INCLUDE_DIRS}) + target_link_libraries(zstd::zstd INTERFACE "${ZSTD_LIBRARY}") endif() diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 66683536b5d..2bbc7016f83 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -3185,6 +3185,11 @@ RAM Cache ``5`` zstd (fast speed, reasonable compression) ======== =================================================================== + ``3``, ``4`` and ``5`` require that |TS| was built with liblzma, lz4 or + libzstd respectively; configuring one that was not compiled in is a fatal + error at startup. ``traffic_layout info`` reports which are available as + ``TS_HAS_LZ4`` and ``TS_HAS_ZSTD``. + Compression runs on task threads. To use more cores for RAM cache compression, increase :ts:cv:`proxy.config.task_threads`. diff --git a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst index d4225040522..ed4b0711d5e 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache-volume.en.rst @@ -135,7 +135,7 @@ a configuration with only one cache volume: :literal:`0`. .. ts:stat:: global proxy.process.cache.volume_0.ram_cache.compress.failure integer :type: counter - Accumulates RAM cache compression failures for this volume: either an entry the compression library rejected with an error, or a whole compression pass skipped because a per-thread compression context could not be allocated. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. Note that a skipped pass is counted once per pass rather than once per entry: because a context allocation failure persists for the life of the thread, such a thread contributes to this counter once per second for each of its stripes until the process is restarted. + Accumulates the number of RAM cache entries the compression library could not compress, for this volume. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. .. ts:stat:: global proxy.process.cache.volume_0.ram_cache.decompress.failure integer :type: counter diff --git a/doc/admin-guide/monitoring/statistics/core/cache.en.rst b/doc/admin-guide/monitoring/statistics/core/cache.en.rst index 6c1c448bec6..27dc8fba579 100644 --- a/doc/admin-guide/monitoring/statistics/core/cache.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/cache.en.rst @@ -98,7 +98,7 @@ Cache .. ts:stat:: global proxy.process.cache.ram_cache.compress.failure integer :type: counter - Accumulates RAM cache compression failures for all volumes: either an entry the compression library rejected with an error, or a whole compression pass skipped because a per-thread compression context could not be allocated. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. Note that a skipped pass is counted once per pass rather than once per entry: because a context allocation failure persists for the life of the thread, such a thread contributes to this counter once per second for each of its stripes until the process is restarted. + Accumulates the number of RAM cache entries the compression library could not compress, for all volumes. Objects that simply did not shrink enough to be worth compressing are not counted, since that is the ordinary outcome for already-compressed content. .. ts:stat:: global proxy.process.cache.ram_cache.decompress.failure integer :type: counter diff --git a/doc/admin-guide/storage/index.en.rst b/doc/admin-guide/storage/index.en.rst index db29b73618d..0b000223e93 100644 --- a/doc/admin-guide/storage/index.en.rst +++ b/doc/admin-guide/storage/index.en.rst @@ -98,20 +98,8 @@ images). This should not be confused with ``Content-Encoding: gzip``, this feature is only present to save space internally in the RAM cache itself. As such, it is completely transparent to the User-Agent. The RAM cache compression is enabled with the option -:ts:cv:`proxy.config.cache.ram_cache.compress`. - -Possible values are: - -======= ============================= -Value Meaning -======= ============================= -0 No compression (*default*) -1 *fastlz* compression -2 *libz* compression -3 *liblzma* compression -4 *lz4* compression -5 *zstd* compression -======= ============================= +:ts:cv:`proxy.config.cache.ram_cache.compress`, which documents the available +codecs and which of them a given build supports. .. _changing-the-size-of-the-ram-cache: diff --git a/doc/developer-guide/cache-architecture/ram-cache.en.rst b/doc/developer-guide/cache-architecture/ram-cache.en.rst index 2fa9c2caa46..fd5b760c647 100644 --- a/doc/developer-guide/cache-architecture/ram-cache.en.rst +++ b/doc/developer-guide/cache-architecture/ram-cache.en.rst @@ -153,7 +153,11 @@ There are 5 algorithms and levels of compression (speed on an Intel Xeon Gold ======= ===== ================= ================== ==================================== Method Level Compression Rate Decompression Rate Notes ======= ===== ================= ================== ==================================== -fastlz 1 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. +fastlz 1/2 452 MB/sec 913 MB/sec Effectively obsolete; prefer lz4. + fastlz_compress() selects + level 2 at 64 KiB and above, + so most objects use it; the + figures here are level 1. Basically free since disk or network will limit first; ~26% final size. libz 6 54 MB/sec 536 MB/sec Effectively obsolete; prefer zstd. diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc index d5dcf3df76b..ac4b56ea675 100644 --- a/src/iocore/cache/Cache.cc +++ b/src/iocore/cache/Cache.cc @@ -866,6 +866,32 @@ ink_cache_init(ts::ModuleVersion v) RecEstablishStaticConfigInt32(cache_config_ram_cache_algorithm, "proxy.config.cache.ram_cache.algorithm"); RecEstablishStaticConfigInt32(cache_config_ram_cache_compress, "proxy.config.cache.ram_cache.compress"); + // Validate here, where the value is read: this runs before any stripe exists + // and the record is RECU_RESTART_TS, so a bad codec cannot reach the RAM + // cache and nothing downstream needs to re-check it. + switch (cache_config_ram_cache_compress) { + case CACHE_COMPRESSION_NONE: + case CACHE_COMPRESSION_FASTLZ: + case CACHE_COMPRESSION_LIBZ: + break; + case CACHE_COMPRESSION_LIBLZMA: +#ifndef HAVE_LZMA_H + Fatal("lzma not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_LZ4: +#ifndef HAVE_LZ4_H + Fatal("lz4 not available for RAM cache compression"); +#endif + break; + case CACHE_COMPRESSION_ZSTD: +#ifndef HAVE_ZSTD_H + Fatal("zstd not available for RAM cache compression"); +#endif + break; + default: + Fatal("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); + } RecEstablishStaticConfigInt32(cache_config_ram_cache_compress_percent, "proxy.config.cache.ram_cache.compress_percent"); cache_config_ram_cache_use_seen_filter = RecGetRecordInt("proxy.config.cache.ram_cache.use_seen_filter").value_or(0); diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index ce94d627f65..299025735ba 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -1668,31 +1668,6 @@ CacheProcessor::cacheInitialized() used_direntries += vol_used_direntries; } - switch (cache_config_ram_cache_compress) { - default: - Fatal("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); - case CACHE_COMPRESSION_NONE: - case CACHE_COMPRESSION_FASTLZ: - break; - case CACHE_COMPRESSION_LIBZ: - break; - case CACHE_COMPRESSION_LIBLZMA: -#ifndef HAVE_LZMA_H - Fatal("lzma not available for RAM cache compression"); -#endif - break; - case CACHE_COMPRESSION_LZ4: -#ifndef HAVE_LZ4_H - Fatal("lz4 not available for RAM cache compression"); -#endif - break; - case CACHE_COMPRESSION_ZSTD: -#ifndef HAVE_ZSTD_H - Fatal("zstd not available for RAM cache compression"); -#endif - break; - } - ts::Metrics::Gauge::store(cache_rsb.ram_cache_bytes_total, total_ram_cache_bytes); ts::Metrics::Gauge::store(cache_rsb.bytes_total, total_cache_bytes); ts::Metrics::Gauge::store(cache_rsb.direntries_total, total_direntries); diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 1084b845255..82146e2753e 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -43,6 +43,8 @@ #endif #ifdef HAVE_ZSTD_H #include +// ZSTD_getErrorCode() and the ZSTD_error_* codes live here, not in zstd.h. +#include #include constexpr int CLFUS_ZSTD_LEVEL = 3; @@ -161,29 +163,7 @@ class RamCacheCLFUSCompressor : public Continuation int RamCacheCLFUSCompressor::mainEvent(int /* event ATS_UNUSED */, Event *e) { - switch (cache_config_ram_cache_compress) { - default: - Warning("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); - case CACHE_COMPRESSION_NONE: - case CACHE_COMPRESSION_FASTLZ: - case CACHE_COMPRESSION_LIBZ: - break; - case CACHE_COMPRESSION_LIBLZMA: -#ifndef HAVE_LZMA_H - Warning("lzma not available for RAM cache compression"); -#endif - break; - case CACHE_COMPRESSION_LZ4: -#ifndef HAVE_LZ4_H - Warning("lz4 not available for RAM cache compression"); -#endif - break; - case CACHE_COMPRESSION_ZSTD: -#ifndef HAVE_ZSTD_H - Warning("zstd not available for RAM cache compression"); -#endif - break; - } + // The codec is validated once in ink_cache_init(), before any cache exists. if (cache_config_ram_cache_compress_percent) { rc->compress_entries(e->ethread); } @@ -200,24 +180,35 @@ static const int bucket_sizes[] = {127, 251, 509, 1021, 203 // cache_config_ram_cache_compress was CACHE_COMPRESSION_NONE at init() time. // That scheduled RamCacheCLFUSCompressor holds a raw back-pointer to this // object and nothing cancels it, so destroying a cache that has one would -// leave it dangling. Cancelling the event here would not be enough: the -// continuation carries no mutex, so it can be running compress_entries() on an -// ET_TASK thread while this destructor runs. Making that safe means giving the -// compressor the stripe mutex and requiring the destructor to hold it, which -// is not worth doing while production never destroys a RamCacheCLFUS -- these -// live for the lifetime of their StripeSM. Unit tests that construct one -// directly must init() with compression off and drive compress_entries() -// synchronously. +// leave it dangling. Cancelling the event here would not be enough either: the +// continuation carries no mutex, so EventProcessor::schedule leaves the event +// with none and it can be running compress_entries() on an ET_TASK thread +// while this destructor runs. Making that safe means giving the compressor its +// own ProxyMutex and cancelling the retained Event under it -- not the stripe +// mutex, because Mutex_unlock() only decrements nthread_holding, so a +// continuation dispatched holding stripe->mutex would keep the stripe locked +// across the codec call and defeat the lock drop in compress_entries(). Not +// worth doing while production never destroys a RamCacheCLFUS -- these live +// for the lifetime of their StripeSM. Unit tests that construct one directly +// must init() with compression off and drive compress_entries() synchronously. RamCacheCLFUS::~RamCacheCLFUS() { // Entries are pool-allocated without running their destructor, so release the // data reference explicitly before returning each one to the allocator, then // free the hash table and the seen filter. - for (auto &lru : this->_lru) { - while (RamCacheCLFUSEntry *e = lru.dequeue()) { - e->data = nullptr; - THREAD_FREE(e, ramCacheCLFUSEntryAllocator, this_thread()); - } + // History entries (lru[1]) hold no data and were never counted. + while (RamCacheCLFUSEntry *e = this->_lru[0].dequeue()) { + this->_bytes -= e->size + entry_overhead; + ts::Metrics::Gauge::decrement(cache_rsb.ram_cache_bytes, e->size); + ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.ram_cache_bytes, e->size); + this->_objects--; + e->data = nullptr; + THREAD_FREE(e, ramCacheCLFUSEntryAllocator, this_thread()); + } + while (RamCacheCLFUSEntry *e = this->_lru[1].dequeue()) { + this->_history--; + e->data = nullptr; + THREAD_FREE(e, ramCacheCLFUSEntryAllocator, this_thread()); } ats_free(this->_bucket); ats_free(this->_seen); @@ -562,10 +553,11 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) // rather than walking every entry -- dropping and retaking the stripe lock // and allocating a compressBound()-sized buffer for each -- only to fail // every time. The entries are left untouched: this says nothing about the - // data, so they stay eligible. Counted once per skipped pass so the - // condition is visible in metrics without flooding them. - ts::Metrics::Counter::increment(cache_rsb.ram_cache_compress_failures); - ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_compress_failures); + // data, so they stay eligible. Deliberately not counted in + // ram_cache.compress.failure: that counter means entries the codec could + // not compress, and incrementing here once per pass would make it climb + // once a second per stripe for the life of the thread. The one-time + // Warning in zstd_cctx() is what reports this condition. return; } #endif @@ -628,11 +620,10 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) MUTEX_UNTAKE_LOCK(stripe->mutex, thread); b = static_cast(ats_malloc(l)); bool failed = false; - // Distinguishes "this thread has no codec context" from "the codec - // rejected this data": the former says nothing about the entry. The - // pass-level check above makes this unreachable for zstd today; it is - // kept so the per-entry handling stays correct on its own. - bool no_context = false; + // Distinguishes a transient, data-independent failure (the codec could + // not allocate its working memory) from the codec rejecting this data. + // Only the latter says anything about the entry. + bool transient = false; switch (ctype) { default: // The bound switch above filtered unknown types; this is unreachable, @@ -646,8 +637,10 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) break; case CACHE_COMPRESSION_LIBZ: { uLongf ll = l; - if ((Z_OK != compress(reinterpret_cast(b), &ll, reinterpret_cast(edata->data()), elen))) { - failed = true; + int rc = compress(reinterpret_cast(b), &ll, reinterpret_cast(edata->data()), elen); + if (Z_OK != rc) { + failed = true; + transient = (rc == Z_MEM_ERROR); } l = static_cast(ll); break; @@ -675,15 +668,17 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) #endif #ifdef HAVE_ZSTD_H case CACHE_COMPRESSION_ZSTD: { + // The pass-level check above already proved this thread has a context, + // and the context is thread_local while the pass never changes thread. ZSTD_CCtx *cctx = zstd_cctx(); - if (cctx == nullptr) { - failed = true; - no_context = true; - break; - } + ink_assert(cctx != nullptr); size_t zret = ZSTD_compress2(cctx, b, l, edata->data(), elen); if (ZSTD_isError(zret)) { failed = true; + // ZSTD_createCCtx() allocates only the context struct; the much + // larger working buffers are allocated on first use and grow with + // the input, so this is the realistic out-of-memory path. + transient = (ZSTD_getErrorCode(zret) == ZSTD_error_memory_allocation); } else { l = static_cast(zret); } @@ -717,10 +712,10 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) if (failed) { ts::Metrics::Counter::increment(cache_rsb.ram_cache_compress_failures); ts::Metrics::Counter::increment(stripe->cache_vol->vol_rsb.ram_cache_compress_failures); - if (no_context) { - // A thread-level allocation failure is not a property of the data, so - // do not record it as permanently incompressible; leave the entry - // eligible for a later pass. + if (transient) { + // An allocation failure inside the codec is not a property of the + // data, so do not record it as permanently incompressible; leave the + // entry eligible for a later pass. ats_free(b); goto Lcontinue; } diff --git a/src/iocore/cache/RamCacheCLFUS.h b/src/iocore/cache/RamCacheCLFUS.h index e52e191e752..76071f3a8bf 100644 --- a/src/iocore/cache/RamCacheCLFUS.h +++ b/src/iocore/cache/RamCacheCLFUS.h @@ -70,6 +70,11 @@ class RamCacheCLFUS : public RamCache RamCacheCLFUS() {} ~RamCacheCLFUS() override; + // Owns raw _bucket/_seen allocations and the pool-allocated entries, so + // copying one would double free all three. + RamCacheCLFUS(const RamCacheCLFUS &) = delete; + RamCacheCLFUS &operator=(const RamCacheCLFUS &) = delete; + // returns 1 on found/stored, 0 on not found/stored, if provided auxkey1 and auxkey2 must match int get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey = 0) override; int put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy = false, uint64_t auxkey = 0) override; diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc index 99c3c1f18f6..c898baefbfe 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -161,11 +161,11 @@ store_compress_get(StripeSM &stripe, int config, const std::vector &payloa Ptr in = make_buffer(payload); uint32_t len = static_cast(payload.size()); - static uint64_t salt = 0; - ++salt; + // A fresh RamCacheCLFUS per call, so one fixed key cannot collide with + // anything. CryptoHash key; - key.u64[0] = 0xc0ffee00 + salt; - key.u64[1] = 0xdeadbeef + salt; + key.u64[0] = 0xc0ffee00; + key.u64[1] = 0xdeadbeef; REQUIRE(rc.put(&key, in.get(), len) == 1); @@ -221,10 +221,13 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca auto payload = incompressible_bytes(256 * 1024); - // Only the backends that actually attempt compression are interesting here; - // skip the NONE case. - auto cases = compression_cases(); - cases.erase(cases.begin()); + // Only the backends that actually attempt compression are interesting here. + std::vector cases; + for (auto const &candidate : compression_cases()) { + if (candidate.config != CACHE_COMPRESSION_NONE) { + cases.push_back(candidate); + } + } const CompressionCase c = GENERATE_REF(from_range(cases)); INFO("compression backend: " << c.name); @@ -233,12 +236,11 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca // Incompressible data is kept verbatim, so a read reports no compression. CHECK(r.hit == RAM_HIT_COMPRESS_NONE); CHECK(r.out == payload); - // And its footprint is unchanged: a regression that stored the expanded + // And the pass never grows the entry: a regression that stored the expanded // "compressed" blob would still read back correctly but would cost memory. - // The payload is a power of two, so the entry carries no buffer padding - // and there is nothing for the pass to legitimately reclaim; a padded - // payload can shrink here by design when CLFUS re-stores it tightly. - CHECK(r.size_after == r.size_before); + // Not an equality check, because re-storing an incompressible entry tightly + // legitimately shrinks a payload that carried buffer padding. + CHECK(r.size_after <= r.size_before); } TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") diff --git a/src/records/unit_tests/test_RecUtils.cc b/src/records/unit_tests/test_RecUtils.cc index 20a85b813ce..6b723df558c 100644 --- a/src/records/unit_tests/test_RecUtils.cc +++ b/src/records/unit_tests/test_RecUtils.cc @@ -26,8 +26,6 @@ #include "../P_RecUtils.h" #include "records/RecordsConfig.h" -#include "iocore/cache/Cache.h" - #include TEST_CASE("recordRangeCheck via RecordValidityCheck", "[librecords][RecUtils]") @@ -228,11 +226,13 @@ TEST_CASE("ram_cache.compress accepts every compression backend", "[librecords][ // The validity range must cover every CACHE_COMPRESSION_* value, otherwise a // documented backend is rejected at load time and silently falls back to the - // default of 0 (no compression). Keep this in sync with Cache.h when a codec - // is added; CACHE_COMPRESSION_ZSTD is currently the largest value. - for (int i = CACHE_COMPRESSION_NONE; i <= CACHE_COMPRESSION_ZSTD; i++) { + // default of 0 (no compression). Kept as a literal rather than including + // iocore/cache/Cache.h, which would point a records test at the cache layer; + // extend it when a codec is added there. + constexpr int largest_compression_type = 5; // CACHE_COMPRESSION_ZSTD + for (int i = 0; i <= largest_compression_type; i++) { INFO("CACHE_COMPRESSION_* value: " << i); REQUIRE(RecordValidityCheck(std::to_string(i).c_str(), record->check, record->regex)); } - REQUIRE_FALSE(RecordValidityCheck(std::to_string(CACHE_COMPRESSION_ZSTD + 1).c_str(), record->check, record->regex)); + REQUIRE_FALSE(RecordValidityCheck(std::to_string(largest_compression_type + 1).c_str(), record->check, record->regex)); } From ebb773b84dd2911cb809d2e0fe88aa8189b4150c Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Wed, 16 Sep 2026 19:31:29 +0000 Subject: [PATCH 13/13] Address non-blocking review: restore zstd shim, test decode failures Restoring the config-mode alias is a fix for my own regression: with CMAKE_FIND_PACKAGE_PREFER_CONFIG the lookup resolves through zstd's config package on a case-insensitive filesystem, which exports zstd::libzstd_shared rather than zstd::zstd, so deleting the alias broke macOS builds at generate time. The new corrupted-frame test reaches the decompression failure paths, which nothing exercised before, and the round-trip checks no longer hand Catch2 two 256 KB operands to stringify, which threw instead of reporting the mismatch. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 29 ++++- NOTICE | 1 - ci/docker/yum/Dockerfile | 2 +- .../cache-architecture/ram-cache.en.rst | 2 +- src/iocore/cache/Cache.cc | 10 +- src/iocore/cache/RamCacheCLFUS.h | 5 + .../cache/unit_tests/test_RamCacheCLFUS.cc | 100 +++++++++++++++++- src/traffic_layout/info.cc | 4 +- 8 files changed, 138 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 30f5a612147..fef88b2d529 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -492,13 +492,36 @@ find_package(ZLIB REQUIRED) # 1.4.0 stabilized the advanced one-shot API (ZSTD_compress2 et al.) used by # the RAM cache; plugins/compress already requires the same floor for # ZSTD_compressStream2, so this is not specific to the cache. -# cmake/FindZSTD.cmake creates the zstd::zstd target this tree links against. find_package(ZSTD 1.4.0) set(HAVE_ZSTD_H ${ZSTD_FOUND}) +# cmake/FindZSTD.cmake creates the zstd::zstd target this tree links against, +# but with CMAKE_FIND_PACKAGE_PREFER_CONFIG the lookup can resolve through +# zstd's own config package instead: on a case-insensitive filesystem the +# ZSTDConfig.cmake CMake searches for matches the zstdConfig.cmake that zstd +# installs. That package exports zstd::libzstd_shared/_static, so alias +# whichever it gave us or the four targets linking zstd::zstd fail at generate +# time. +if(ZSTD_FOUND AND NOT TARGET zstd::zstd) + foreach(_zstd_target zstd::libzstd_shared zstd::libzstd_static zstd::libzstd) + if(TARGET ${_zstd_target}) + add_library(zstd_zstd INTERFACE) + target_link_libraries(zstd_zstd INTERFACE ${_zstd_target}) + add_library(zstd::zstd ALIAS zstd_zstd) + break() + endif() + endforeach() + unset(_zstd_target) + if(NOT TARGET zstd::zstd) + message(WARNING "zstd found but it exports no target this build can use; building without zstd") + set(HAVE_ZSTD_H FALSE) + endif() +endif() + # 1.7.0 (r129) introduced the current compression API, including -# LZ4_compress_default(), which the RAM cache uses. -find_package(LZ4 1.7.0) +# LZ4_compress_default(), which the RAM cache uses; 1.7.5 is the floor for +# LZ4_versionString(), which traffic_layout reports. +find_package(LZ4 1.7.5) set(HAVE_LZ4_H ${LZ4_FOUND}) # ncurses is used in traffic_top diff --git a/NOTICE b/NOTICE index 38e089ab93e..7fb1e7a6358 100644 --- a/NOTICE +++ b/NOTICE @@ -149,4 +149,3 @@ algorithms (Wojciech Muła and Daniel Lemire's vectorized base64, and aqrit's combined standard/URL-safe classifier) and lookup tables originate there. simdutf: https://github.com/simdutf/simdutf (Apache-2.0 / MIT / BSL-1.0) Copyright (c) 2021 The simdutf authors - diff --git a/ci/docker/yum/Dockerfile b/ci/docker/yum/Dockerfile index 6070c5218d5..c97237b21ab 100644 --- a/ci/docker/yum/Dockerfile +++ b/ci/docker/yum/Dockerfile @@ -52,7 +52,7 @@ RUN yum -y update; \ # Devel packages that ATS needs yum -y install openssl-devel expat-devel pcre-devel libcap-devel hwloc-devel libunwind-devel \ xz-devel libcurl-devel ncurses-devel jemalloc-devel GeoIP-devel luajit-devel brotli-devel \ - ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel libzstd-devel lz4-devel \ + ImageMagick-devel ImageMagick-c++-devel hiredis-devel zlib-devel zstd-devel lz4-devel \ perl-ExtUtils-MakeMaker perl-Digest-SHA perl-URI; \ # This is for autest stuff yum -y install python3 httpd-tools procps-ng nmap-ncat \ diff --git a/doc/developer-guide/cache-architecture/ram-cache.en.rst b/doc/developer-guide/cache-architecture/ram-cache.en.rst index fd5b760c647..a28d7f148b8 100644 --- a/doc/developer-guide/cache-architecture/ram-cache.en.rst +++ b/doc/developer-guide/cache-architecture/ram-cache.en.rst @@ -74,7 +74,7 @@ compressed_len Compressed length of the object. compressed Compression type, or ``none`` if no compression. Possible types are: *fastlz*, *libz*, *liblzma*, *lz4* and *zstd*. incompressible Flag indicating that content cannot be compressed (true), or that - it mat be compressed (false). + it may be compressed (false). copy Whether or not this object should be copied in and copied out (e.g. HTTP HDR). LRU link diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc index ac4b56ea675..1fad04027b5 100644 --- a/src/iocore/cache/Cache.cc +++ b/src/iocore/cache/Cache.cc @@ -876,21 +876,23 @@ ink_cache_init(ts::ModuleVersion v) break; case CACHE_COMPRESSION_LIBLZMA: #ifndef HAVE_LZMA_H - Fatal("lzma not available for RAM cache compression"); + Fatal("proxy.config.cache.ram_cache.compress is %d (liblzma), but this build has no liblzma support", + cache_config_ram_cache_compress); #endif break; case CACHE_COMPRESSION_LZ4: #ifndef HAVE_LZ4_H - Fatal("lz4 not available for RAM cache compression"); + Fatal("proxy.config.cache.ram_cache.compress is %d (lz4), but this build has no lz4 support", cache_config_ram_cache_compress); #endif break; case CACHE_COMPRESSION_ZSTD: #ifndef HAVE_ZSTD_H - Fatal("zstd not available for RAM cache compression"); + Fatal("proxy.config.cache.ram_cache.compress is %d (zstd), but this build has no zstd support", + cache_config_ram_cache_compress); #endif break; default: - Fatal("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); + Fatal("proxy.config.cache.ram_cache.compress has unknown value %d", cache_config_ram_cache_compress); } RecEstablishStaticConfigInt32(cache_config_ram_cache_compress_percent, "proxy.config.cache.ram_cache.compress_percent"); cache_config_ram_cache_use_seen_filter = RecGetRecordInt("proxy.config.cache.ram_cache.use_seen_filter").value_or(0); diff --git a/src/iocore/cache/RamCacheCLFUS.h b/src/iocore/cache/RamCacheCLFUS.h index 76071f3a8bf..d4c13836d02 100644 --- a/src/iocore/cache/RamCacheCLFUS.h +++ b/src/iocore/cache/RamCacheCLFUS.h @@ -102,6 +102,11 @@ class RamCacheCLFUS : public RamCache int _ncompressed = 0; RamCacheCLFUSEntry *_compressed = nullptr; // first uncompressed lru[0] entry + // Lets the unit tests reach a stored entry so the decompression failure + // paths in get() can be exercised; see unit_tests/test_RamCacheCLFUS.cc. + // Nothing in the product uses this. + friend struct RamCacheCLFUSTestAccess; + void _resize_hashtable(); void _victimize(RamCacheCLFUSEntry *e); void _move_compressed(RamCacheCLFUSEntry *e); diff --git a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc index c898baefbfe..46d1026151c 100644 --- a/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc +++ b/src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc @@ -29,6 +29,7 @@ #include "iocore/cache/Cache.h" #include "tscore/ink_config.h" +#include #include #include #include @@ -38,6 +39,20 @@ int cache_vols = 1; bool reuse_existing_cache = false; +// Reaches into RamCacheCLFUS to find a stored entry; declared a friend there. +struct RamCacheCLFUSTestAccess { + static RamCacheCLFUSEntry * + find_entry(RamCacheCLFUS &rc, const CryptoHash &key) + { + for (RamCacheCLFUSEntry *e = rc._bucket[key.slice32(3) % rc._nbuckets].head; e != nullptr; e = e->hash_link.next) { + if (e->key == key) { + return e; + } + } + return nullptr; + } +}; + namespace { @@ -137,6 +152,22 @@ incompressible_bytes(std::size_t len) return bytes; } +// Comparing two 256 KB vectors with CHECK() makes Catch2 stringify both +// operands, which throws before it can report anything useful. Report the +// offset of the first difference instead, so a genuine round-trip failure +// names the byte. +std::size_t +first_difference(const std::vector &lhs, const std::vector &rhs) +{ + std::size_t common = std::min(lhs.size(), rhs.size()); + for (std::size_t i = 0; i < common; i++) { + if (lhs[i] != rhs[i]) { + return i; + } + } + return common; +} + struct RoundtripResult { int hit = 0; int64_t size_before = 0; // rc.size() after put, before the compression pass @@ -203,7 +234,8 @@ TEST_CASE("CLFUS compressible objects roundtrip cleanly", "[cache][ramcache][com RoundtripResult r = store_compress_get(stripe, c.config, payload); CHECK(r.hit == c.expected_hit); - CHECK(r.out == payload); + CHECK(r.out.size() == payload.size()); + CHECK(first_difference(r.out, payload) == payload.size()); if (c.config != CACHE_COMPRESSION_NONE) { // The feature's contract is that compression saves memory, not merely // that the entry is tagged compressed. @@ -235,7 +267,8 @@ TEST_CASE("CLFUS incompressible objects fall back to uncompressed storage", "[ca // Incompressible data is kept verbatim, so a read reports no compression. CHECK(r.hit == RAM_HIT_COMPRESS_NONE); - CHECK(r.out == payload); + CHECK(r.out.size() == payload.size()); + CHECK(first_difference(r.out, payload) == payload.size()); // And the pass never grows the entry: a regression that stored the expanded // "compressed" blob would still read back correctly but would cost memory. // Not an equality check, because re-storing an incompressible entry tightly @@ -262,7 +295,68 @@ TEST_CASE("CLFUS single-byte payload roundtrips", "[cache][ramcache][compress]") // too-small guard, the incompressible marking, or storing the bytes verbatim // -- the object must survive and read back uncompressed. CHECK(r.hit == RAM_HIT_COMPRESS_NONE); - CHECK(r.out == payload); + CHECK(r.out.size() == payload.size()); + CHECK(first_difference(r.out, payload) == payload.size()); +} + +TEST_CASE("CLFUS reports a corrupted compressed entry rather than serving it", "[cache][ramcache][compress]") +{ + CacheDisk disk; + init_disk(disk); + StripeSM stripe{&disk, 10, 0}; + CacheVol cache_vol; + wire_stripe(stripe, cache_vol); + + // Only backends that actually store a compressed blob can have one corrupted. + std::vector cases; + for (auto const &candidate : compression_cases()) { + if (candidate.config != CACHE_COMPRESSION_NONE) { + cases.push_back(candidate); + } + } + const CompressionCase c = GENERATE_REF(from_range(cases)); + INFO("compression backend: " << c.name); + + cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE; + cache_config_ram_cache_compress_percent = 100; + cache_config_ram_cache_use_seen_filter = 0; + + RamCacheCLFUS rc; + rc.init(1 << 20, &stripe); + + auto payload = compressible_bytes(256 * 1024); + uint32_t len = static_cast(payload.size()); + Ptr in = make_buffer(payload); + + CryptoHash key; + key.u64[0] = 0xc0ffee00; + key.u64[1] = 0xdeadbeef; + + REQUIRE(rc.put(&key, in.get(), len) == 1); + + cache_config_ram_cache_compress = c.config; + rc.compress_entries(this_ethread()); + + RamCacheCLFUSEntry *e = RamCacheCLFUSTestAccess::find_entry(rc, key); + REQUIRE(e != nullptr); + // The pass must actually have compressed it, or there is nothing to corrupt. + REQUIRE(e->flag_bits.compressed != 0); + REQUIRE(e->compressed_len > 0); + + // Overwrite the whole stored blob. Every codec here rejects this either + // outright or by producing the wrong length, which is what get() checks. + std::memset(e->data->data(), 0xff, e->compressed_len); + + int64_t before = ts::Metrics::Counter::load(cache_rsb.ram_cache_decompress_failures); + + Ptr ret; + int hit = rc.get(&key, &ret); + + // A corrupted entry is a miss, is counted, and is dropped rather than + // handed to the caller. + CHECK(hit == 0); + CHECK(ts::Metrics::Counter::load(cache_rsb.ram_cache_decompress_failures) == before + 1); + CHECK(RamCacheCLFUSTestAccess::find_entry(rc, key) == nullptr); } // A backend that is not compiled in silently disappears from the parametrized diff --git a/src/traffic_layout/info.cc b/src/traffic_layout/info.cc index 6bc3ee67e06..d15fb3011b5 100644 --- a/src/traffic_layout/info.cc +++ b/src/traffic_layout/info.cc @@ -269,8 +269,8 @@ produce_versions(bool json) print_var("zstd", undef, json); #endif #ifdef HAVE_LZ4_H - print_var("lz4", LBW().print("{}", LZ4_VERSION_STRING).view(), json); - print_var("lz4.run", LBW().print("{}", LZ4_versionString()).view(), json); + // Runtime version, matching what the zstd line above reports. + print_var("lz4", LBW().print("{}", LZ4_versionString()).view(), json); #else print_var("lz4", undef, json); #endif