crimson/osd: sending EVENT_DISCONNECT becomes implementation detail of Watch.

In contrast to ceph-osd crimson sends CEPH_WATCH_EVENT_DISCONNECT directly
from the timeout handler and after CEPH_WATCH_EVENT_NOTIFY_COMPLETE.
This simplifies the Watch::remove() interface as callers aren't obliged
anymore to decide whether EVENT_DISCONNECT needs to be send or not -- it
becomes an implementation detail of Watch.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 955e977932a3531bdb1506c6cc89f5196cbf68d0..cf030cbe87d0340ed46c1a0b20e94ce0d4142755 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -218,8 +218,6 @@ OpsExecuter::watch_ierrorator::future<> OpsExecuter::do_op_watch_subop_unwatch(
 
   struct disconnect_ctx_t {
     ObjectContext::watch_key_t key;
-    bool send_disconnect{ false };
-
     disconnect_ctx_t(const OSDOp& osd_op, const ExecutableMessage& msg)
       : key(osd_op.op.watch.cookie, msg.get_reqid().name) {
     }
@@ -240,7 +238,7 @@ OpsExecuter::watch_ierrorator::future<> OpsExecuter::do_op_watch_subop_unwatch(
         return seastar::do_with(std::move(nh.mapped()),
                          [ctx](auto&& watcher) {
           logger().info("op_effect: disconnect watcher {}", ctx.key);
-          return watcher->remove(ctx.send_disconnect);
+          return watcher->remove();
         });
       } else {
         logger().info("op_effect: disconnect failed to find watcher {}", ctx.key);

diff --git a/src/crimson/osd/watch.cc b/src/crimson/osd/watch.cc
index ccdec827712c83d1c65566817bb85d75f96de226..a5a3c46254039723289cbf3d0c46cfbed8f07ad9 100644 (file)
--- a/src/crimson/osd/watch.cc
+++ b/src/crimson/osd/watch.cc
@@ -155,19 +155,20 @@ void Watch::got_ping(utime_t)
   }
 }
 
-seastar::future<> Watch::remove(const bool send_disconnect)
+seastar::future<> Watch::remove()
 {
   logger().info("{}", __func__);
-  auto disconnected = send_disconnect ? send_disconnect_msg()
-                                      : seastar::now();
-  return std::move(disconnected).then([this] {
-    return seastar::do_for_each(in_progress_notifies,
-      [this_shared=shared_from_this()] (auto notify) {
-        return notify->remove_watcher(this_shared);
-      }).then([this] {
-        discard_state();
-        return seastar::now();
-      });
+  // in contrast to ceph-osd crimson sends CEPH_WATCH_EVENT_DISCONNECT directly
+  // from the timeout handler and _after_ CEPH_WATCH_EVENT_NOTIFY_COMPLETE.
+  // this simplifies the Watch::remove() interface as callers aren't obliged
+  // anymore to decide whether EVENT_DISCONNECT needs to be send or not -- it
+  // becomes an implementation detail of Watch.
+  return seastar::do_for_each(in_progress_notifies,
+    [this_shared=shared_from_this()] (auto notify) {
+      return notify->remove_watcher(this_shared);
+    }).then([this] {
+      discard_state();
+      return seastar::now();
     });
 }
 
@@ -181,8 +182,11 @@ void Watch::cancel_notify(const uint64_t notify_id)
 
 void Watch::do_watch_timeout(Ref<PG> pg)
 {
-  pg->get_shard_services().start_operation<WatchTimeoutRequest>(
+  auto [op, fut] = pg->get_shard_services().start_operation<WatchTimeoutRequest>(
     shared_from_this(), pg);
+  std::ignore = std::move(fut).then([op=std::move(op), this] {
+    return send_disconnect_msg();
+  });
 }
 
 bool notify_reply_t::operator<(const notify_reply_t& rhs) const

diff --git a/src/crimson/osd/watch.h b/src/crimson/osd/watch.h
index fd5b0d1a6eb814c0aeec5568223a5879a344641f..24d4d315accca92ab306c271925ba4af95b83d36 100644 (file)
--- a/src/crimson/osd/watch.h
+++ b/src/crimson/osd/watch.h
@@ -75,7 +75,7 @@ public:
   }
   void got_ping(utime_t);
 
-  seastar::future<> remove(bool send_disconnect);
+  seastar::future<> remove();
 
   /// Call when notify_ack received on notify_id
   seastar::future<> notify_ack(