From: Jesse Williamson Date: Wed, 8 Jul 2026 23:31:52 +0000 (-0700) Subject: Add support for reverse-order selection. X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a4a5f15e48b76b72ac6fabf36f8bec034db9d64f;p=ceph.git Add support for reverse-order selection. Updates tests, examples. Assisted-by: Codex:GPT-5 Signed-off-by: Jesse Williamson --- diff --git a/src/rgw/fdb/EXAMPLES.md b/src/rgw/fdb/EXAMPLES.md index f06f24d6890..9f1521813a8 100644 --- a/src/rgw/fdb/EXAMPLES.md +++ b/src/rgw/fdb/EXAMPLES.md @@ -21,6 +21,21 @@ namespace lfdb = ceph::libfdb; using namespace std::string_literals; ``` +## Running Tests And Benchmarks + +From the build directory, run the libfdb tests with: + +```sh +./bin/unittest_fdb +./bin/unittest_fdb_ceph +``` + +Benchmarks are hidden from default test runs. Run all libfdb benchmarks with: + +```sh +./bin/unittest_fdb_ceph "[benchmark]" +``` + ## General Recipes ```cpp @@ -169,6 +184,32 @@ std::ranges::copy(lfdb::pair_generator(dbh, lfdb::select { "person/" }), std::inserter(people, std::end(people))); ``` +To get results in reverse order, set the reverse_order property in the selector: + +```cpp +auto people = lfdb::select { "person/" }; +people.options.reverse_order = true; + +for (const auto& [key, value] : lfdb::pair_generator(dbh, people)) { + /* process results from high keys to low keys */ +} +``` + +While block_generator() provides a way to get blocks of results, it also has different +request behavior than pair_generator(); it may therefore be useful to group pair_generator()'s +output into chunks. One way to do that is with a chunk_view: + +```cpp +// Stream groups of 100: +auto keys = lfdb::pair_generator(dbh, lfdb::select { "key_" }); + +for (const auto& chunk : keys | std::views::chunk(100)) { + for (const auto& [key, value] : chunk) { + // ... + } +} +``` + ## Block Generator For very large prefix scans, use the same one-argument selector with diff --git a/src/rgw/fdb/base.h b/src/rgw/fdb/base.h index b62ce6e855c..3efaad24b03 100644 --- a/src/rgw/fdb/base.h +++ b/src/rgw/fdb/base.h @@ -246,18 +246,16 @@ struct select final { std::string begin_key, end_key; - public: - // We'll eventually need a way to get settings into the base library from the binding layer; there - // are a few ways we could do it, this is one I'm mulling over; do not use this right now. mutable struct { int stride = 0; // "unlimited" - // Some parts of the documentation claim FDB_STREAMING_MODE_ITERATOR is the default, other parts - // don't... examples tend to use FDB_STREAMING_MODE_WANT_ALL, but they operate on fairly small amounts - // of data. It's pretty hard to understand what the Right Thing(TM) to do is, the sure the answer may - // evolve as I learn more, but for now this setting at least means it's "plumbed through" (this - // particular mode starts with small batches and then grows to larger increments as more data is sent), - // even though this isn't really used at the moment: + bool reverse_order = false; // should we return results in reverse order? + + // Some parts of the documentation claim FDB_STREAMING_MODE_ITERATOR is the default, other parts + // don't... examples tend to use FDB_STREAMING_MODE_WANT_ALL, but they operate on fairly small amounts + // of data. It's pretty hard to understand what the Right Thing(TM) to do is, the sure the answer may + // evolve as I learn more; this particular mode starts with small batches and then grows to larger + // increments as more data is sent and seems to be generally well-behaved: FDBStreamingMode streaming_mode = FDB_STREAMING_MODE_ITERATOR; } options; @@ -744,7 +742,9 @@ inline bool retry_after_error(ceph::libfdb::transaction_handle& txn, const fdb_e } // Convert FDBKey array into something useful: -inline std::vector as_select_seq(const FDBKey* const xs, const int n) +inline std::vector as_select_seq(const FDBKey* const xs, + const int n, + const ceph::libfdb::select& parent) { std::vector out; @@ -757,10 +757,13 @@ inline std::vector as_select_seq(const FDBKey* const xs, c const auto& fst = std::ranges::begin(dyad)[0]; const auto& snd = std::ranges::begin(dyad)[1]; - out.push_back({ + ceph::libfdb::select split { { (const char *)fst.key, static_cast(fst.key_length) }, { (const char *)snd.key, static_cast(snd.key_length) } - }); + }; + + split.options = parent.options; + out.push_back(std::move(split)); } return out; @@ -774,12 +777,11 @@ inline future_value get_range_future_from_transaction(ceph::libfdb::transaction& const auto& begin_key = selection.begin_key; const auto& end_key = selection.end_key; - // Hook for getting settings into here through the selector: - const auto& streaming_mode = selection.options.streaming_mode; + const auto& options = selection.options; // The documentation makes this stuff about as clear as mud... read VERY carefully // when you fiddle with these: - int begin_or_eq = (1 == iteration) ? 0 : 1; + const int begin_or_eq = (not options.reverse_order and 1 < iteration) ? 1 : 0; const int begin_offset = 1; const int end_or_eq = 0; const int end_offset = 1; @@ -798,14 +800,14 @@ inline future_value get_range_future_from_transaction(ceph::libfdb::transaction& end_offset, // end offset (a shift AFTER end is matched) // How should results be grouped/chunked: - 0, // limit (0 == unlimited) + options.stride, // limit (0 == unlimited) 0, // target bytes (0 == unlimited) - streaming_mode, // streaming mode (e.g.: FDB_STREAMING_MODE_WANT_ALL) + options.streaming_mode, // streaming mode (e.g.: FDB_STREAMING_MODE_WANT_ALL) iteration, // iteration # (produced side effect) // Other options: 0, // 0 unless this IS a snapshot read - 0 // reverse: should items come in reverse order? + options.reverse_order // should items come in reverse order? )); } @@ -836,7 +838,7 @@ inline std::vector locate_split_points( })); if (not should_retry) { - return as_select_seq(keys, nkeys); + return as_select_seq(keys, nkeys, selector); } } @@ -868,7 +870,9 @@ inline std::generator> generate_FDB_pairs(transacti if (more_available) { // Make the first part of the range for the new search equal to the last one from the old search: const auto& last_key = result.back(); - key_range.begin_key = std::string_view((const char *)last_key.key, last_key.key_length); + auto& cursor = key_range.options.reverse_order ? key_range.end_key : key_range.begin_key; + + cursor = std::string_view((const char *)last_key.key, last_key.key_length); } co_yield result; diff --git a/src/rgw/fdb/interface.h b/src/rgw/fdb/interface.h index d5677e0f18f..c06d65dab07 100644 --- a/src/rgw/fdb/interface.h +++ b/src/rgw/fdb/interface.h @@ -432,6 +432,7 @@ inline transactor make_transactor(database_handle dbh, const transaction_options // Basic Generators: namespace ceph::libfdb { +// For ordinary range scans, pair_generator() is usually the right default: template inline auto pair_generator(ceph::libfdb::transaction_handle txn, ceph::libfdb::select key_range) -> std::generator> @@ -450,7 +451,17 @@ inline auto pair_generator(const ceph::libfdb::database_handle& dbh, ceph::libfd } // Note: block_generator() uses split planning to tackle large sets; use pair_generator() for -// direct scans. This is meant to be straightforward and easy-to-understand-- hence, there's not +// direct scans. +// +// What block_generator() gives you: +// - avoids one huge transaction getting too old +// - gives caller block-at-a-time processing +// - can bound memory and transaction duration better than a monolithic scan +// +// Note: block_generator() was originally parallel, and could be again, but preliminary benchmarking +// showed it to be a significant performance impediment. The database must be truly large to see benefits. +// +// Note: This is meant to be straightforward and easy-to-understand-- hence, there's not // a recover strategy or other things (you can replay the entire query)-- as new needs arise, this // can be made more flexible via selector options, dynamic range-splitting, etc., but so far there // has been no need: diff --git a/src/test/rgw/test_fdb.cc b/src/test/rgw/test_fdb.cc index 23c59dd01b0..35078a49fe5 100644 --- a/src/test/rgw/test_fdb.cc +++ b/src/test/rgw/test_fdb.cc @@ -273,6 +273,20 @@ TEST_CASE("check selectors", "[fdb][rgw]") { CHECK(make_key(0) == out.front().first); CHECK(make_key(nentries - 1) == out.back().first); + SECTION("reverse order") { + auto reverse_all = select_all; + reverse_all.options.reverse_order = true; + + out.clear(); + lfdb::get(dbh, reverse_all, std::back_inserter(out)); + + REQUIRE(nentries == out.size()); + CHECK(make_key(nentries - 1) == out.front().first); + CHECK(make_key(0) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::greater {}, + &std::pair::first)); + } + lfdb::set(dbh, "keyx", "outside"); out.clear(); lfdb::get(dbh, lfdb::select { "key_" }, std::back_inserter(out)); @@ -402,20 +416,89 @@ TEST_CASE("basic generators", "[fdb]") { const auto kvs_in = write_monotonic_kvs(dbh, nkeys); REQUIRE(nkeys == kvs_in.size()); - SECTION("pair_generator, kv pair return") { - std::map out; + SECTION("pair_generator forward") { + std::vector> out; - // pair_generator returns key-value pairs, keeping the specified transaction (or implicitly created one) - // alive until exhausted (note that this may cause the transaction to expire if approaching 5s or so): + // pair_generator returns key-value pairs: for(auto&& kvp : lfdb::pair_generator(dbh, lfdb::select { make_key(0), make_key(nkeys) })) - out.emplace(kvp); + out.emplace_back(std::move(kvp)); + CAPTURE(nkeys); + CAPTURE(out.size()); REQUIRE(nkeys == out.size()); // Be sure we captured the head and the tail: if(0 < nkeys) { - CHECK(out.contains(make_key(0))); - CHECK(out.contains(make_key(nkeys - 1))); + CAPTURE(out.front().first); + CAPTURE(out.back().first); + CHECK(make_key(0) == out.front().first); + CHECK(make_key(nkeys - 1) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::less {}, + &std::pair::first)); + } + } + + SECTION("pair_generator reverse") { + auto selector = lfdb::select { make_key(0), make_key(nkeys) }; + selector.options.reverse_order = true; + + std::vector> out; + std::ranges::copy(lfdb::pair_generator(dbh, selector), std::back_inserter(out)); + + CAPTURE(nkeys); + CAPTURE(out.size()); + REQUIRE(nkeys == out.size()); + + if(0 < nkeys) { + CAPTURE(out.front().first); + CAPTURE(out.back().first); + CHECK(make_key(nkeys - 1) == out.front().first); + CHECK(make_key(0) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::greater {}, + &std::pair::first)); + } + } + + SECTION("pair_generator forward, paged") { + auto selector = lfdb::select { make_key(0), make_key(nkeys) }; + selector.options.stride = 5; // one of the most prime of prime numbers + + std::vector> out; + std::ranges::copy(lfdb::pair_generator(dbh, selector), std::back_inserter(out)); + + CAPTURE(nkeys); + CAPTURE(out.size()); + REQUIRE(nkeys == out.size()); + + if(0 < nkeys) { + CAPTURE(out.front().first); + CAPTURE(out.back().first); + CHECK(make_key(0) == out.front().first); + CHECK(make_key(nkeys - 1) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::less {}, + &std::pair::first)); + } + } + + SECTION("pair_generator reverse, paged") { + auto selector = lfdb::select { make_key(0), make_key(nkeys) }; + selector.options.reverse_order = true; + selector.options.stride = 5; // one of the most prime of prime numbers + + std::vector> out; + std::ranges::copy(lfdb::pair_generator(dbh, selector), std::back_inserter(out)); + + CAPTURE(nkeys); + CAPTURE(out.size()); + REQUIRE(nkeys == out.size()); + + if(0 < nkeys) { + CAPTURE(out.front().first); + CAPTURE(out.back().first); + CHECK(make_key(nkeys - 1) == out.front().first); + CHECK(make_key(0) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::greater {}, + &std::pair::first)); } } @@ -432,6 +515,8 @@ TEST_CASE("basic generators", "[fdb]") { std::ranges::copy(gen, std::inserter(out, std::end(out))); + CAPTURE(nkeys); + CAPTURE(out.size()); REQUIRE(nkeys == out.size()); if(0 < nkeys) { @@ -679,9 +764,9 @@ SCENARIO("options", "[fdb]") { { FDB_DB_OPTION_LOCATION_CACHE_SIZE, 200'000 } }, { { FDB_NET_OPTION_TRACE_ENABLE, lfdb::option_flag } }); - auto dbh1 = lfdb::create_database("fishing for databass!", // name - { { FDB_DB_OPTION_LOCATION_CACHE_SIZE, 200'000 } }, // database options - { { FDB_NET_OPTION_TRACE_ENABLE, lfdb::option_flag } }); // network options + auto dbh1 = lfdb::create_database("fishing for databass!", // name + { { FDB_DB_OPTION_LOCATION_CACHE_SIZE, 200'000 } }, // database options + { { FDB_NET_OPTION_TRACE_ENABLE, lfdb::option_flag } }); // network options auto txn = lfdb::make_transaction(dbh0, { { FDB_TR_OPTION_READ_YOUR_WRITES_DISABLE, lfdb::option_flag } }); diff --git a/src/test/rgw/test_fdb_ceph.cc b/src/test/rgw/test_fdb_ceph.cc index 764157379a0..c9b5705c7c2 100644 --- a/src/test/rgw/test_fdb_ceph.cc +++ b/src/test/rgw/test_fdb_ceph.cc @@ -339,8 +339,18 @@ auto tier_generator(ceph::libfdb::database_handle dbh, ceph::libfdb::select sele } // namespace ceph::libfdb +struct ordered_block final : std::vector> +{ + using std::vector>::vector; + + void emplace(std::pair&& value) + { + emplace_back(std::move(value)); + } +}; + -TEST_CASE("generators", "[fdb][rgw]") { +TEST_CASE("block_generator", "[fdb][rgw]") { /* This is a generator for large queries that are pretty much expected to exceed the FoundationDB five-second transaction limit: */ @@ -351,7 +361,7 @@ exceed the FoundationDB five-second transaction limit: */ populate_monotonic(j, nkeys); - SECTION("retrieve all") { + SECTION("forward") { auto selector = lfdb::select { "key_" }; size_t total = 0; @@ -359,12 +369,38 @@ exceed the FoundationDB five-second transaction limit: */ total += block.size(); } + CAPTURE(nkeys); + CAPTURE(total); CHECK(nkeys == total); } + + SECTION("reverse") { + auto selector = lfdb::select { "key_" }; + selector.options.reverse_order = true; + + std::vector> out; + + for(const auto& block : lfdb::block_generator(j, selector)) { + std::ranges::copy(block, std::back_inserter(out)); + } + + CAPTURE(nkeys); + CAPTURE(out.size()); + REQUIRE(nkeys == out.size()); + + if(0 < nkeys) { + CAPTURE(out.front().first); + CAPTURE(out.back().first); + CHECK(make_key(nkeys - 1) == out.front().first); + CHECK(make_key(0) == out.back().first); + CHECK(std::ranges::is_sorted(out, std::ranges::greater {}, + &std::pair::first)); + } + } } } -TEST_CASE("generators", "[benchmark]") { +TEST_CASE("generators", "[.benchmark][benchmark]") { const size_t nkeys = GENERATE(0, 1, 1'000); // 5'000, 10'000, 50'000, 250'000, 1'000'000); fmt::println("generator benchmark, nkeys = {}", nkeys); @@ -388,10 +424,150 @@ TEST_CASE("generators", "[benchmark]") { }; } +TEST_CASE("read path benchmarks", "[.benchmark][benchmark]") { + const size_t nkeys = GENERATE(0, 1, 100, 1'000, 10'000); // 250'000, 500'000, 1'000'000 + + fmt::println("read path benchmark, nkeys = {}", nkeys); + + // Set up bulk data for all of the different test reads. + janitor dbh; + populate_monotonic(dbh, nkeys); + + const auto selector = lfdb::select { "key_" }; + + // Track enough work to prevent the benchmarked reads from being optimized away: + struct { + size_t total = 0; + size_t bytes = 0; + bool ok = true; + std::string error; + + void reset() + { + total = 0; + bytes = 0; + ok = true; + error.clear(); + } + + void add(std::string_view value) + { + ++total; + bytes += value.size(); + } + + } read_tally; + + auto mark_failure = [&read_tally](const std::exception& e) { + read_tally.ok = false; + read_tally.error = e.what(); + }; + + auto add_kv = [&read_tally](const auto& kv) { + read_tally.add(kv.second); + }; + + auto key_indices = [nkeys] { + return std::views::iota(0u, static_cast(nkeys)); + }; + + auto expected_bytes = [&] { + size_t bytes = 0; + + std::ranges::for_each(key_indices(), [&bytes](const auto n) { + bytes += make_value(n).size(); + }); + + return bytes; + }(); + + auto require_expected = [nkeys, expected_bytes](const auto& tally) { + if(not tally.ok) { + WARN(tally.error); + return; + } + + REQUIRE(nkeys == tally.total); + REQUIRE(expected_bytes == tally.bytes); + }; + + BENCHMARK_ADVANCED("single-key get, one shared transaction")(Catch::Benchmark::Chronometer meter) { + meter.measure([&] { + read_tally.reset(); + + try { + auto txn = lfdb::make_transaction(dbh); + + for(const auto n : key_indices()) { + std::string out; + + if (lfdb::get(txn, make_key(n), out, lfdb::commit_after_op::no_commit)) { + read_tally.add(out); + } + } + } catch(const ceph::libfdb::libfdb_exception& e) { + mark_failure(e); + } + }); + + require_expected(read_tally); + }; + + BENCHMARK_ADVANCED("single-key get, implicit transaction per key")(Catch::Benchmark::Chronometer meter) { + meter.measure([&] { + read_tally.reset(); + + try { + for(const auto n : key_indices()) { + std::string out; + + if (lfdb::get(dbh, make_key(n), out)) { + read_tally.add(out); + } + } + } catch(const ceph::libfdb::libfdb_exception& e) { + mark_failure(e); + } + }); + + require_expected(read_tally); + }; + + BENCHMARK_ADVANCED("pair generator read all")(Catch::Benchmark::Chronometer meter) { + meter.measure([&] { + read_tally.reset(); + + try { + std::ranges::for_each(lfdb::pair_generator(dbh, selector), add_kv); + } catch(const ceph::libfdb::libfdb_exception& e) { + mark_failure(e); + } + }); + + require_expected(read_tally); + }; + + BENCHMARK_ADVANCED("block generator read all")(Catch::Benchmark::Chronometer meter) { + meter.measure([&] { + read_tally.reset(); + + try { + std::ranges::for_each(lfdb::block_generator(dbh, selector), [&](const auto& block) { + std::ranges::for_each(block, add_kv); + }); + } catch(const ceph::libfdb::libfdb_exception& e) { + mark_failure(e); + } + }); + + require_expected(read_tally); + }; +} + // Note that these are disabled for regular test runs. Use // unittest_fdb_ceph "simple benchmarks" // ...to run: -TEST_CASE("simple benchmarks", "[benchmark]") { +TEST_CASE("simple benchmarks", "[.benchmark][benchmark]") { using namespace std::ranges; using std::for_each;