crimson/seastore: cleanup collection-lock leftovers

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

diff --git a/src/crimson/os/seastore/ordering_handle.h b/src/crimson/os/seastore/ordering_handle.h
index e4b375416e868698ece54fc46d1ee76c300774f7..957b39f62438eea7c6bfd683094926be3f122fc7 100644 (file)
--- a/src/crimson/os/seastore/ordering_handle.h
+++ b/src/crimson/os/seastore/ordering_handle.h
@@ -123,45 +123,12 @@ struct OrderingHandle {
   // we can easily optimize this dynalloc out as all concretes are
   // supposed to have exactly the same size.
   std::unique_ptr<OperationProxy> op;
-  seastar::shared_mutex *collection_ordering_lock = nullptr;
-
-  seastar::lowres_clock::time_point lock_acquire_time{};
-  seastar::lowres_clock::duration lock_hold_time{0};
 
   // in the future we might add further constructors / template to type
   // erasure while extracting the location of tracking events.
   OrderingHandle(std::unique_ptr<OperationProxy> op) : op(std::move(op)) {}
   OrderingHandle(OrderingHandle &&other)
-    : op(std::move(other.op)),
-      collection_ordering_lock(other.collection_ordering_lock),
-      lock_acquire_time(other.lock_acquire_time),
-      lock_hold_time(other.lock_hold_time) {
-    other.collection_ordering_lock = nullptr;
-  }
-
-  seastar::future<> take_collection_lock(seastar::shared_mutex &mutex) {
-    ceph_assert(!collection_ordering_lock);
-    collection_ordering_lock = &mutex;
-    return collection_ordering_lock->lock();
-  }
-
-  void set_lock_acquire_time(seastar::lowres_clock::time_point tp) {
-    lock_acquire_time = tp;
-  }
-
-  seastar::lowres_clock::duration get_lock_hold_time() const {
-    return lock_hold_time;
-  }
-
-  void maybe_release_collection_lock() {
-    if (collection_ordering_lock) {
-      if (lock_acquire_time != seastar::lowres_clock::time_point{}) {
-        lock_hold_time = seastar::lowres_clock::now() - lock_acquire_time;
-      }
-      collection_ordering_lock->unlock();
-      collection_ordering_lock = nullptr;
-    }
-  }
+    : op(std::move(other.op)) {}
 
   template <typename T>
   seastar::future<> enter(T &t) {
@@ -175,10 +142,6 @@ struct OrderingHandle {
   seastar::future<> complete() {
     return op->complete();
   }
-
-  ~OrderingHandle() {
-    maybe_release_collection_lock();
-  }
 };
 
 inline OrderingHandle get_dummy_ordering_handle() {

diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index 1102bfbe768a8aadc2f653a4cb4dbfcafc0ca160..8baa7951c5ef01c7774e237a0ae86cbdeee828e9 100644 (file)
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -215,8 +215,6 @@ void SeaStore::Shard::register_metrics(store_index_t store_index)
   );
 
   std::pair<txn_stage_t, sm::label_instance> labels_by_stage[] = {
-    {txn_stage_t::COLLOCK_WAIT,          sm::label_instance("stage", "collock_wait")},
-    {txn_stage_t::COLLOCK_HOLD,          sm::label_instance("stage", "collock_hold")},
     {txn_stage_t::THROTTLER_WAIT,        sm::label_instance("stage", "throttler_wait")},
     {txn_stage_t::BUILD,                 sm::label_instance("stage", "build")},
     {txn_stage_t::BUILD_GET_ONODE,       sm::label_instance("stage", "build_get_onode")},
@@ -921,14 +919,13 @@ seastar::future<> SeaStore::report_stats()
          calc_conflicts(io_total.read_num, io_total.repeat_read_num),
          calc_conflicts(io_total.get_bg_num(), io_total.get_repeat_bg_num()));
     INFO("trans outstanding: {},{},{},{} "
-         "per-shard: {:.2f}({:.2f},{:.2f},{:.2f},{:.2f},{:.2f}),{:.2f},{:.2f},{:.2f}",
+         "per-shard: {:.2f}({:.2f},{:.2f},{:.2f},{:.2f}),{:.2f},{:.2f},{:.2f}",
          io_total.pending_io_num,
          io_total.pending_read_num,
          io_total.pending_bg_num,
          io_total.pending_flush_num,
          (double)io_total.pending_io_num/seastar::smp::count,
          (double)io_total.starting_io_num/seastar::smp::count,
-         (double)io_total.waiting_collock_io_num/seastar::smp::count,
          (double)io_total.waiting_throttler_io_num/seastar::smp::count,
          (double)io_total.processing_inlock_io_num/seastar::smp::count,
          (double)io_total.processing_postlock_io_num/seastar::smp::count,
@@ -940,7 +937,6 @@ seastar::future<> SeaStore::report_stats()
     for (const auto &s : shard_io_stats) {
       oss_pending << s.pending_io_num
                  << "(" << s.starting_io_num
-                 << "," << s.waiting_collock_io_num
                  << "," << s.waiting_throttler_io_num
                  << "," << s.processing_inlock_io_num
                  << "," << s.processing_postlock_io_num
@@ -1889,8 +1885,6 @@ seastar::future<> SeaStore::Shard::run_one_batch(
     const std::array<
       std::pair<txn_stage_t, seastar::lowres_clock::duration>, STAGE_MAX>
       stage_samples = {{
-        {txn_stage_t::COLLOCK_WAIT,          seastar::lowres_clock::duration::zero()},
-        {txn_stage_t::COLLOCK_HOLD,          seastar::lowres_clock::duration::zero()},
         {txn_stage_t::THROTTLER_WAIT,        throttler_wait},
         {txn_stage_t::BUILD,                 ctx.build_time},
         {txn_stage_t::BUILD_GET_ONODE,       ctx.get_onode_time},
@@ -3051,7 +3045,7 @@ shard_stats_t SeaStore::Shard::get_io_stats(
     };
     INFO("iops={:.2f},{:.2f},{:.2f}({:.2f},{:.2f},{:.2f},{:.2f}),{:.2f} "
          "conflicts={:.2f},{:.2f},{:.2f}({:.2f},{:.2f},{:.2f},{:.2f}) "
-         "outstanding={}({},{},{},{},{}),{},{},{}",
+         "outstanding={}({},{},{},{}),{},{},{}",
          // iops
          ret.io_num/seconds,
          ret.read_num/seconds,
@@ -3072,7 +3066,6 @@ shard_stats_t SeaStore::Shard::get_io_stats(
          // outstanding
          ret.pending_io_num,
          ret.starting_io_num,
-         ret.waiting_collock_io_num,
          ret.waiting_throttler_io_num,
          ret.processing_inlock_io_num,
          ret.processing_postlock_io_num,

diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h
index 7ad4a20d1dd5a7ed49c0ffca30684732af0ed6c5..605e444b8e7206a5631f01ac0031c1a1aaba790f 100644 (file)
--- a/src/crimson/os/seastore/seastore.h
+++ b/src/crimson/os/seastore/seastore.h
@@ -49,9 +49,7 @@ enum class op_type_t : uint8_t {
 };
 
 enum class txn_stage_t : uint8_t {
-    COLLOCK_WAIT = 0,  // waiting on the collection ordering_lock
-    COLLOCK_HOLD,      // collection ordering_lock held (acquire -> release at prepare_record)
-    THROTTLER_WAIT,    // waiting for a throttler slot
+    THROTTLER_WAIT = 0, // waiting for a throttler slot
     BUILD,             // building the transaction (_do_transaction_step loop)
     BUILD_GET_ONODE,   // onode_manager get/get_or_create calls within BUILD
     SUBMIT_TOTAL,      // the whole submit_transaction (pipeline + journal write)
@@ -71,8 +69,6 @@ public:
   SeastoreCollection(T&&... args) :
     FuturizedCollection(std::forward<T>(args)...) {}
 
-  seastar::shared_mutex ordering_lock;
-
   struct batch_entry_t {
     ceph::os::Transaction txn;
     seastar::promise<> pr;

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 082baeacea342c8498d101bfe2148f58d518ec70..58b23da381475ab2424a46c36768b46b02751c36 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -3293,7 +3293,6 @@ struct shard_stats_t {
   uint64_t repeat_io_num = 0;
   uint64_t pending_io_num = 0;
   uint64_t starting_io_num = 0;
-  uint64_t waiting_collock_io_num = 0;
   uint64_t waiting_throttler_io_num = 0;
   uint64_t processing_inlock_io_num = 0;
   uint64_t processing_postlock_io_num = 0;
@@ -3336,7 +3335,6 @@ struct shard_stats_t {
     repeat_io_num += o.repeat_io_num;
     pending_io_num += o.pending_io_num;
     starting_io_num += o.starting_io_num;
-    waiting_collock_io_num += o.waiting_collock_io_num;
     waiting_throttler_io_num += o.waiting_throttler_io_num;
     processing_inlock_io_num += o.processing_inlock_io_num;
     processing_postlock_io_num += o.processing_postlock_io_num;

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index a8d28e276f3ce06b9e9a70bfc1222fca13d7256b..b4bfd50a2a5777cb08ad3fc7ac93e7df2339c5a2 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -703,7 +703,6 @@ TransactionManager::do_submit_transaction(
   tref.get_phase_durations().prepare_record +=
     std::chrono::steady_clock::now() - prepare_record_start;
 
-  tref.get_handle().maybe_release_collection_lock();
   if (tref.get_src() == Transaction::src_t::MUTATE) {
     --(shard_stats.processing_inlock_io_num);
     ++(shard_stats.processing_postlock_io_num);
@@ -749,7 +748,6 @@ seastar::future<> TransactionManager::flush(OrderingHandle &handle)
   }).then([this, &handle] {
     return handle.enter(write_pipeline.prepare);
   }).then([this, &handle] {
-    handle.maybe_release_collection_lock();
     return journal->flush(handle);
   }).then([FNAME, &handle] {
     SUBDEBUG(seastore_t, "H{} completed", (void*)&handle);