crimson/os/seastore/onode: measure onode-tree lookup cost

Before optimizing the onode lookup paths we should measure the existing
cost if each possible optimization.

The per-node search is already binary search(see search_result_bs_t
binary_search). Possible costs are:
- per-level node traversal
- STAGE_STRING ns/oid memcmp (rbd has long shared oid prefixes)

Adding the following stats to be exposed as seastar-metrics (not logs).

Keep the new sampling per-comparison overhead out unless needed for
measurment runs.

Candidate optimization leads from the numbers:
* cross-node string/prefix dedup (marked as TODOs in stage.h) this could
  result in cheaper compares for shared (rbd) prefixes.
* shrink onode_layout_t - marked as onode.h TODO.
  stop inlining oi and ss to get more objects per leaf.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

diff --git a/src/crimson/CMakeLists.txt b/src/crimson/CMakeLists.txt
index ff5c366f03678c498d0d54e886cc5da3fe454511..2a97044226b21cc83e53f57ceed9427f97ab0f82 100644 (file)
--- a/src/crimson/CMakeLists.txt
+++ b/src/crimson/CMakeLists.txt
@@ -1,6 +1,12 @@
 add_library(crimson::cflags INTERFACE IMPORTED)
 set(crimson_cflag_definitions "WITH_CRIMSON=1")
 
+option(WITH_CRIMSON_DETAILED_SAMPLING
+  "Enable detailed onode-tree sampling metrics in crimson-seastore" OFF)
+if(WITH_CRIMSON_DETAILED_SAMPLING)
+  list(APPEND crimson_cflag_definitions "CRIMSON_DETAILED_SAMPLING=1")
+endif()
+
 set_target_properties(crimson::cflags PROPERTIES
   INTERFACE_COMPILE_DEFINITIONS "${crimson_cflag_definitions}"
   INTERFACE_LINK_LIBRARIES Seastar::seastar)

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
index 9d78d87a8cde8b9787a139b5098ba69fc7467b8f..0a4f890bb2b5f8a44486dcbd290a37a8a51b2762 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -319,6 +319,9 @@ level_t Node::level() const
 eagain_ifuture<Node::search_result_t> Node::lower_bound(
     context_t c, const key_hobj_t& key)
 {
+#ifdef CRIMSON_DETAILED_SAMPLING
+  ++c.t.get_onode_tree_stats().lookup_count;
+#endif
   return seastar::do_with(
     MatchHistory(), [this, c, &key](auto& history) {
       return lower_bound_tracked(c, key, history);
@@ -332,6 +335,9 @@ eagain_ifuture<std::pair<Ref<tree_cursor_t>, bool>> Node::insert(
     value_config_t vconf,
     Ref<Node>&& this_ref)
 {
+#ifdef CRIMSON_DETAILED_SAMPLING
+  ++c.t.get_onode_tree_stats().lookup_count;
+#endif
   return seastar::do_with(
     MatchHistory(), [this, c, &key, vconf,
                      this_ref = std::move(this_ref)] (auto& history) mutable {
@@ -1265,7 +1271,15 @@ eagain_ifuture<Node::search_result_t>
 InternalNode::lower_bound_tracked(
     context_t c, const key_hobj_t& key, MatchHistory& history)
 {
+#ifdef CRIMSON_DETAILED_SAMPLING
+  auto& tstats = c.t.get_onode_tree_stats();
+  ++tstats.nodes_visited;
+  auto cmp0 = key.ncmp_str;
+  auto result = impl->lower_bound(key, history);
+  tstats.string_cmp_count += key.ncmp_str - cmp0;
+#else
   auto result = impl->lower_bound(key, history);
+#endif
   return get_or_track_child(c, result.position, result.p_value->value
   ).si_then([c, &key, &history](auto child) {
     // XXX(multi-type): pass result.mstat to child
@@ -1954,7 +1968,15 @@ LeafNode::lower_bound_tracked(
     context_t c, const key_hobj_t& key, MatchHistory& history)
 {
   key_view_t index_key;
+#ifdef CRIMSON_DETAILED_SAMPLING
+  auto& tstats = c.t.get_onode_tree_stats();
+  ++tstats.nodes_visited;
+  auto cmp0 = key.ncmp_str;
+  auto result = impl->lower_bound(key, history, &index_key);
+  tstats.string_cmp_count += key.ncmp_str - cmp0;
+#else
   auto result = impl->lower_bound(key, history, &index_key);
+#endif
   Ref<tree_cursor_t> cursor;
   if (result.position.is_end()) {
     assert(!result.p_value);

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/key_layout.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/key_layout.h
index ca8659d8c0f809a24c05b8083f86e0b31803d897..562acc8cc9bef24fb109f460d01ed0a70b21d963 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/key_layout.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/key_layout.h
@@ -406,6 +406,10 @@ bool is_valid_key(const Key& key);
  */
 class key_hobj_t {
  public:
+  // number of ns/oid comparisons this query key participated in during lookup.
+  // Only incremented under CRIMSON_DETAILED_SAMPLING; otherwise stays zero.
+  mutable uint64_t ncmp_str = 0;
+
   explicit key_hobj_t(const ghobject_t& _ghobj) {
     if (_ghobj.is_max()) {
       ghobj = _MAX_OID();
@@ -920,6 +924,12 @@ auto operator<=>(const T& key, const shard_pool_crush_t& target) {
 
 template <IsFullKey T>
 auto operator<=>(const T& key, const ns_oid_view_t& target) {
+#ifdef CRIMSON_DETAILED_SAMPLING
+  // only key_hobj_t carries the counter
+  if constexpr (requires { key.ncmp_str; }) {
+    ++key.ncmp_str;
+  }
+#endif
   auto ret = key.nspace() <=> string_view_masked_t{target.nspace};
   if (ret != 0)
     return ret;

diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index d816527047601c3b9011e1d6c055e5f92f94de9a..ea7b8898df01149c46a6c0d995dccd925aca36d5 100644 (file)
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -249,6 +249,54 @@ void SeaStore::Shard::register_metrics(store_index_t store_index)
     );
   }
 
+  metrics.add_group(
+    "onode_tree",
+    {
+      sm::make_counter(
+        "onode_lookups",
+        [this] { return stats.onode_lookups; },
+        sm::description("onode-tree find + insert search"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_counter(
+        "onode_lookup_nodes",
+        [this] { return stats.onode_lookup_nodes; },
+        sm::description("nodes searched across onode-tree"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_counter(
+        "onode_lookup_str_cmp_count",
+        [this] { return stats.onode_str_cmp_count; },
+        sm::description("ns/oid key comparisons during onode-tree ops"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_counter(
+        "onode_inserts",
+        [this] { return stats.onode_inserts; },
+        sm::description("onode-tree key inserts"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_counter(
+        "onode_updates",
+        [this] { return stats.onode_updates; },
+        sm::description("onode-tree value updates"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_counter(
+        "onode_erases",
+        [this] { return stats.onode_erases; },
+        sm::description("onode-tree key erases"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+      sm::make_gauge(
+        "onode_extents_delta",
+        [this] { return stats.onode_extents_delta; },
+        sm::description("net onode-tree extents added(+)/removed(-)"),
+        {sm::label_instance("shard_store_index", std::to_string(store_index))}
+      ),
+    }
+  );
+
   metrics.add_group(
     "seastore",
     {
@@ -1786,6 +1834,7 @@ seastar::future<> SeaStore::Shard::do_transaction_no_callbacks(
   add_latency_sample(
     op_type_t::DO_TRANSACTION,
     std::chrono::steady_clock::now() - ctx.begin_timestamp);
+  add_onode_tree_sample(ctx.transaction->get_onode_tree_stats());
 
   throttler.put();
 

diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h
index 93fb8bbf0a51df5eb804e3baa26db98f60ae9714..34ac5e060c31df6a89bbc631594ca2cc42818e2a 100644 (file)
--- a/src/crimson/os/seastore/seastore.h
+++ b/src/crimson/os/seastore/seastore.h
@@ -465,8 +465,25 @@ public:
       std::array<seastar::metrics::histogram, LAT_MAX> op_lat;
       seastar::metrics::histogram conflict_replays;
       std::array<seastar::metrics::histogram, STAGE_MAX> stage_lat;
+      uint64_t onode_lookups = 0;      // tree lookups
+      uint64_t onode_lookup_nodes = 0; // nodes searched (~depth/lookup)
+      uint64_t onode_str_cmp_count = 0;// ns/oid memcmp calls
+      uint64_t onode_inserts = 0;
+      uint64_t onode_updates = 0;
+      uint64_t onode_erases = 0;
+      int64_t  onode_extents_delta = 0;
     } stats;
 
+    void add_onode_tree_sample(const Transaction::tree_stats_t& ts) {
+      stats.onode_lookups        += ts.lookup_count;
+      stats.onode_lookup_nodes   += ts.nodes_visited;
+      stats.onode_str_cmp_count  += ts.string_cmp_count;
+      stats.onode_inserts        += ts.num_inserts;
+      stats.onode_updates        += ts.num_updates;
+      stats.onode_erases         += ts.num_erases;
+      stats.onode_extents_delta  += ts.extents_num_delta;
+    }
+
     seastar::metrics::histogram& get_latency(
       op_type_t op_type) {
       assert(static_cast<std::size_t>(op_type) < stats.op_lat.size());

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index 3c2be49db9ac09bcec7eb31efe9263a9815ad531..8b32be30ba4ed5a05c41257516cefb698e6bd8a6 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -567,13 +567,19 @@ public:
     uint64_t num_erases = 0;
     uint64_t num_updates = 0;
     int64_t extents_num_delta = 0;
+    uint64_t lookup_count = 0;
+    uint64_t nodes_visited = 0;
+    uint64_t string_cmp_count = 0;
 
     bool is_clear() const {
       return (depth == 0 &&
               num_inserts == 0 &&
               num_erases == 0 &&
               num_updates == 0 &&
-             extents_num_delta == 0);
+             extents_num_delta == 0 &&
+              lookup_count == 0 &&
+              nodes_visited == 0 &&
+              string_cmp_count == 0);
     }
   };
   tree_stats_t& get_onode_tree_stats() {