From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Wed, 7 Apr 2021 11:41:39 +0000 (+0000)
Subject: crimson/osd: sending EVENT_DISCONNECT becomes implementation detail of Watch.
X-Git-Tag: v17.1.0~1984^2~3
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=bf6404e2b116d16b9d04f881b86948cdc523c59c;p=ceph-ci.git

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 955e977932a..cf030cbe87d 100644
--- 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 ccdec827712..a5a3c462540 100644
--- 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 fd5b0d1a6eb..24d4d315acc 100644
--- 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(