Backport/squid 19.2.4 cobaltcore storage/rgw kms cache - #3
Closed
irq0 wants to merge 28 commits into
Closed
Conversation
if async_wait() can race with complete() across threads, the yield_waiter's handler_state needs to be protected by a mutex. add an async_wait() overload for unique_lock that behaves like condition_variable::wait(): the lock is released immediately before suspending, and reacquired immediately before calling its completion handler Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 604fad5)
also adds an empty() function so it's easier to specify its precondition Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 165f2ec)
modeled after std::call_once() to guarantee that racing callers wait for the initial caller to finish. the main differences here are * support for coroutine callers to suspend instead of blocking while waiting for the initial caller, and * the wrapped function must return a value, which is cached and returned to all callers Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit 3cd3c11)
A cache data structure for values that need to be retrieved form outside systems (e.g Key Management Systems). Features: - Thread safe, optimized for concurrent lookups and cache hits - Entry TTL expiration - Cache replacement strategy tuned to "web" workloads (SIEVE) - Performance Counters on hit, miss, expire, size, capacity, clears Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 9618827)
Add Google benchmark [0] based micro benchmarks for Cache/LRU implementations in the Ceph code base. [0] https://github.com/google/benchmark Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 8354a97)
Benchmark: - Linux Kernel Key Retention Service (kernel keystore) [0] - memfd_secret(2) - plain memory Tests: - Random reads - (keystore) Write, Read, Remove [0] https://docs.kernel.org/security/keys/core.html Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit a4b8d7d)
Add RAII wrapper around the Linux Key Retention Service add_key(2), keyctl_read(3), keyctl_invalidate(3) Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 0307119)
To allow RGW threads to share possession over process keyring keys the keyring must be created before a child thread adds keys. Since we only use the process keyring for KMS cache secrets, only initialize the keyring if it is enabled on startup. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 0000467)
Add caching section to the RGW Encryption docs. Add cache settings to the RGW configuration reference. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit a88f029)
Goal: Support multiple backends and faking / mocking for testing. Add abstract classes Keyring (factory) and KeyringSecret. Add "Unsupported" implementation for non-Linux platforms. Add a get_best factory function that currently returns the LinuxKeyring impl on Linux or Unsupported elsewhere. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 35f2bb4)
Add SSE Key Management System secrets cache to RGW. It is common to have secrets shared by many if not all objects in a bucket. Without RGW-side caching every PUT/GET will cause a request to an external KSM. This not only adds load to the KSM, but also slows down read and writes. Combine WebCache, ceph::async::call_once and LinuxKeyringSecret into KMSCache. WebCache stores async::once_result to wrap results of a KMS secret fetch to mitigate cache stampedes (concurrent cache requests to the same key coalesce into one). The retrieved secrets are stored in the Linux kernel key retention service (LinuxKeyringSecret) for safe keeping and retrial by subsequent requests. KMSCache adds a TTL reaper and life cycle. Cache values and error handling: The cache stores positive fetch results, permanent errors (e.g key does not exists) and transient errors (e.g fetch timeout). Each with a different TTL. Unit tests to cover cached / uncached KMS retrieve and runtime cache disable via config. Add perf counter `kms_fetch_lat` to track KMS fetch request latency and error counters to track permanent, transient and key store errors. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> Fixes: https://tracker.ceph.com/issues/68524 On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit b6186fc)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit aa4ead3)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit fb5bbc7)
KMS backends Barbican, Vault KV, and KMIP have a static key per key_id. However, with Vault Transit, each object has a unique DEK wrapped by the transit key. Keying th cache with key_id in Transit mode results in only the first DEK to be cached for all subsequent objects. Fix this by appending a hash of the wrapped DEK to the cache key. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 4d7c556)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 78f2ec3)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 898cd5b)
sieve_expire_erase_unmutexed did not update the sieve hand passed as advertised. Make it return the updated hand and use that to update the global _sieve_hand in expire_erase Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 3518aaf)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit f96f7a6)
Reset reaper state to monostate in the async and threaded case. Fixes a possible use after free in the async reaper case. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit e104372)
1. Don't delete the KMS cache before draining/joining the frontend coroutine threads. They may still depend on the KMS cache. 2. Stop the TTL reaper early to get it off the coroutine pool. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 62ba03f)
The testing backend uses a 'keysel' attribute to derive a per object key from the KEK in the config. A single key_id with distinct keysel has different keys and need to be cached as such. Add the keysel to the cache key id to handle these collisions. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com (cherry picked from commit 12de72b)
Signed-off-by: Casey Bodley <cbodley@redhat.com> (cherry picked from commit be4ddd7)
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com
Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com
c826e15 common/perf_counters: track maximum increment value for LONGRUNAVG perf intruduced a return parameter change regression. Since this only affects kms cache unit tests, apply the reversal there. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com> On-behalf-of: SAP marcel.lauhoff@sap.com
new dout macros use fmt::print() to write output directly to the
underlying ostream. this enables libfmt's printf-style formatting syntax
for ceph log output, without the string allocation overhead of:
dout(10) << fmt::format("str={} count={}", str, count) << dendl;
which becomes:
dout_fmt(10, "str={} count={}", str, count);
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit a58d07a)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
documentation
irq0
removed
documentation
senolcolak
pushed a commit
that referenced
this pull request
Aug 6, 2026
Previously, error messages passed to luaL_error() were formatted using
std::string concatenation. Since luaL_error() never returns (it throws
a Lua exception via longjmp), the allocated std::string memory was
leaked, as detected by AddressSanitizer:
```
Direct leak of 105 byte(s) in 1 object(s) allocated from:
#0 0x7fc5f1921a2d in operator new(unsigned long) /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:86
#1 0x563bd89144c7 in std::__new_allocator<char>::allocate(unsigned long, void const*) /usr/include/c++/15.1.1/bits/new_allocator.h:151
#2 0x563bd89144c7 in std::allocator<char>::allocate(unsigned long) /usr/include/c++/15.1.1/bits/allocator.h:203
#3 0x563bd89144c7 in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&, unsigned long) /usr/include/c++/15.1.1/bits/alloc_traits.h:614
#4 0x563bd89144c7 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_allocate(std::allocator<char>&, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.h:142
#5 0x563bd89144c7 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned long&, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:164
#6 0x563bd896ae1b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:363
#7 0x563bd896b256 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_append(char const*, unsigned long) /usr/include/c++/15.1.1/bits/basic_string.tcc:455
#8 0x563bd896b2bb in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::append(char const*) /usr/include/c++/15.1.1/bits/basic_string.h:1585
#9 0x563bd943c2f2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > std::operator+<char, std::char_traits<char>, std::allocator<char> >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&, char const*) /usr/include/c++/15.1.1/bits/basic_string.h:3977
ceph#10 0x563bd943c2f2 in rgw::lua::lua_state_guard::runtime_hook(lua_State*, lua_Debug*) /home/kefu/dev/ceph/src/rgw/rgw_lua_utils.cc:245
ceph#11 0x7fc5f139f8ef (/usr/lib/liblua.so.5.4+0xe8ef) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#12 0x7fc5f139fbfe (/usr/lib/liblua.so.5.4+0xebfe) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#13 0x7fc5f13b26fe (/usr/lib/liblua.so.5.4+0x216fe) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#14 0x7fc5f139f581 (/usr/lib/liblua.so.5.4+0xe581) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#15 0x7fc5f139b735 (/usr/lib/liblua.so.5.4+0xa735) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#16 0x7fc5f139ba8f (/usr/lib/liblua.so.5.4+0xaa8f) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#17 0x7fc5f139f696 in lua_pcallk (/usr/lib/liblua.so.5.4+0xe696) (BuildId: b7533e2973d4b0d82e10fc29973ec5a8d355d2b8)
ceph#18 0x563bd8a925ef in rgw::lua::request::execute(rgw::sal::Driver*, RGWREST*, OpsLogSink*, req_state*, RGWOp*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/kefu/dev/ceph/src/rgw/rgw_lua_request.cc:824
ceph#19 0x563bd8952e3d in TestRGWLua_LuaRuntimeLimit_Test::TestBody() /home/kefu/dev/ceph/src/test/rgw/test_rgw_lua.cc:1628
ceph#20 0x563bd8a37817 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/kefu/dev/ceph/src/googletest/googletest/src/gtest.cc:2653
ceph#21 0x563bd8a509b5 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/kefu/dev/ceph/build/bin/unittest_rgw_lua+0x11199b5) (BuildId: b2628caba5290d882d25f7bea166f058b682bc85)`
```
This change replaces std::string formatting with stack-allocated buffer
and std::to_chars() to eliminate the memory leak.
Note: We cannot format int64_t directly through luaL_error() because
lua_pushfstring() does not support long long or int64_t format specifiers,
even in Lua 5.4 (see https://www.lua.org/manual/5.4/manual.html#lua_pushfstring).
Since libstdc++ uses int64_t for std::chrono::milliseconds::rep, we use
std::to_chars() for safe, efficient conversion without heap allocation.
The maximum runtime limit was a configuration introduced by 3e3cb15.
Fixes: https://tracker.ceph.com/issues/71595
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
(cherry picked from commit d8adc49)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes cobaltcore-dev/cloud-storage#463
Backport file
release-management/backports/<uuid>.mdadded (or updated) by this PRidin frontmatter matches the filename stemprovenance.upstream_prsset (orprovenance.type: otherwith another:block)Stage-B risk (filled per
release-management/backports/RISK-RUBRIC.md)blast— cosmetic / availability / data-lossconflict— clean / trivial / substantivecoverage— strong / partial / weakhighband)Merge
Land this via
just merge-backport PR=<N>— do not click the green merge button. The just recipeconstructs the merge commit's
Backport-Idtrailer.