librados: unwatch doesn't need the oid argument

The handle is like a file handle: there is internal state that is getting
freed and the user needn't provide it again.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h
index 200b0acd35482752dbab4b5f595fe25c847195db..770c3742470a0a9bf4f5bd933af5b4541eac014b 100644 (file)
--- a/src/include/rados/librados.h
+++ b/src/include/rados/librados.h
@@ -1977,11 +1977,11 @@ CEPH_RADOS_API int rados_watch(rados_ioctx_t io, const char *o, uint64_t ver,
  * @param arg opaque value to pass to the callback
  * @returns 0 on success, negative error code on failure
  */
-int rados_watch2(rados_ioctx_t io, const char *o, uint64_t *handle,
-                rados_watchcb2_t watchcb,
-                rados_watchfailcb_t watchfailcb,
-                rados_watcherrcb_t watcherrcb,
-                void *arg);
+CEPH_RADOS_API int rados_watch2(rados_ioctx_t io, const char *o, uint64_t *handle,
+                               rados_watchcb2_t watchcb,
+                               rados_watchfailcb_t watchfailcb,
+                               rados_watcherrcb_t watcherrcb,
+                               void *arg);
 
 /**
  * Unregister an interest in an object
@@ -1990,12 +1990,24 @@ int rados_watch2(rados_ioctx_t io, const char *o, uint64_t *handle,
  * watch. This should be called to clean up unneeded watchers.
  *
  * @param io the pool the object is in
- * @param o the name of the watched object
+ * @param o the name of the watched object (ignored)
  * @param handle which watch to unregister
  * @returns 0 on success, negative error code on failure
  */
-CEPH_RADOS_API int rados_unwatch(rados_ioctx_t io, const char *o,
-                                 uint64_t handle);
+CEPH_RADOS_API int rados_unwatch(rados_ioctx_t io, const char *o, uint64_t handle)
+  __attribute__((deprecated));
+
+/**
+ * Unregister an interest in an object
+ *
+ * Once this completes, no more notifies will be sent to us for this
+ * watch. This should be called to clean up unneeded watchers.
+ *
+ * @param io the pool the object is in
+ * @param handle which watch to unregister
+ * @returns 0 on success, negative error code on failure
+ */
+CEPH_RADOS_API int rados_unwatch2(rados_ioctx_t io, uint64_t handle);
 
 /**
  * Sychronously notify watchers of an object

diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp
index 9c845221e0a02f35eb798f6ed4f8c81d94f1e8b0..bbd92f37afa902a3cdd3b37cd86af466e0497199 100644 (file)
--- a/src/include/rados/librados.hpp
+++ b/src/include/rados/librados.hpp
@@ -941,7 +941,7 @@ namespace librados
     // watch/notify
     int watch(const std::string& o, uint64_t *handle,
              librados::WatchCtx2 *ctx);
-    int unwatch(const std::string& o, uint64_t handle);
+    int unwatch(uint64_t handle);
     int notify(const std::string& o,   ///< object
               bufferlist& bl,         ///< optional broadcast payload
               uint64_t timeout_ms,    ///< timeout (in ms)
@@ -961,6 +961,8 @@ namespace librados
              librados::WatchCtx *ctx) __attribute__ ((deprecated));
     int notify(const std::string& o, uint64_t ver, bufferlist& bl)
       __attribute__ ((deprecated));
+    int unwatch(const std::string& o, uint64_t handle)
+      __attribute__ ((deprecated));
 
     /**
      * Set allocation hint for an object

diff --git a/src/librados/IoCtxImpl.cc b/src/librados/IoCtxImpl.cc
index c5e296f7e1de6ed9bb3b59e07b92bbedd5d52d9c..0d2a89552e2c85dd56a8d9b0ad41a13751667069 100644 (file)
--- a/src/librados/IoCtxImpl.cc
+++ b/src/librados/IoCtxImpl.cc
@@ -1070,7 +1070,7 @@ int librados::IoCtxImpl::watch(const object_t& oid,
 
   if (r < 0) {
     lock->Lock();
-    client->unregister_watch_notify_callback(*cookie); // destroys wc
+    client->unregister_watch_notify_callback(*cookie, NULL); // destroys wc
     lock->Unlock();
   }
 
@@ -1092,7 +1092,13 @@ int librados::IoCtxImpl::notify_ack(
   return 0;
 }
 
-int librados::IoCtxImpl::unwatch(const object_t& oid, uint64_t cookie)
+int librados::IoCtxImpl::watch_check(uint64_t cookie)
+{
+  Mutex::Locker(*lock);
+  return client->watch_check(cookie);
+}
+
+int librados::IoCtxImpl::unwatch(uint64_t cookie)
 {
   bufferlist inbl, outbl;
 
@@ -1104,7 +1110,12 @@ int librados::IoCtxImpl::unwatch(const object_t& oid, uint64_t cookie)
   version_t ver;
   lock->Lock();
 
-  client->unregister_watch_notify_callback(cookie);
+  object_t oid;
+  r = client->unregister_watch_notify_callback(cookie, &oid);
+  if (r < 0) {
+    lock->Unlock();
+    return r;
+  }
 
   ::ObjectOperation wr;
   prepare_assert_ops(&wr);
@@ -1182,7 +1193,7 @@ int librados::IoCtxImpl::notify(const object_t& oid, bufferlist& bl,
   ldout(client->cct, 10) << __func__ << " completed notify (linger op " << wc->linger_id << "), unregistering" << dendl;
 
   lock->Lock();
-  client->unregister_watch_notify_callback(cookie);   // destroys wc
+  client->unregister_watch_notify_callback(cookie, NULL);   // destroys wc
   lock->Unlock();
 
   set_sync_op_version(objver);

diff --git a/src/librados/IoCtxImpl.h b/src/librados/IoCtxImpl.h
index 6b3c7b5330ae21a05b17361286e2af96eecd866b..e786b4f4b9c22321f9683992ebb01451d8a9cc8f 100644 (file)
--- a/src/librados/IoCtxImpl.h
+++ b/src/librados/IoCtxImpl.h
@@ -199,7 +199,7 @@ struct librados::IoCtxImpl {
   void set_sync_op_version(version_t ver);
   int watch(const object_t& oid, uint64_t *cookie, librados::WatchCtx *ctx,
            librados::WatchCtx2 *ctx2);
-  int unwatch(const object_t& oid, uint64_t cookie);
+  int unwatch(uint64_t cookie);
   int notify(const object_t& oid, bufferlist& bl, uint64_t timeout_ms,
             bufferlist *preplybl, char **preply_buf, size_t *preply_buf_len);
   int notify_ack(const object_t& oid, uint64_t notify_id, uint64_t cookie,

diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc
index 2be7db1faf2f392c0178ed59a28b3fc523746e9f..0fcc3a444ac1b5717e3014dc2bc65d9b9f0e4ed0 100644 (file)
--- a/src/librados/RadosClient.cc
+++ b/src/librados/RadosClient.cc
@@ -653,25 +653,30 @@ void librados::RadosClient::register_watch_notify_callback(
   watch_notify_info[wc->cookie] = wc;
 }
 
-void librados::RadosClient::unregister_watch_notify_callback(uint64_t cookie)
+int librados::RadosClient::unregister_watch_notify_callback(uint64_t cookie,
+                                                           object_t *poid)
 {
   ldout(cct,10) << __func__ << " cookie " << cookie << dendl;
   assert(lock.is_locked_by_me());
-  map<uint64_t, WatchNotifyInfo *>::iterator iter =
-    watch_notify_info.find(cookie);
-  if (iter != watch_notify_info.end()) {
-    WatchNotifyInfo *ctx = iter->second;
-    if (ctx->linger_id)
-      objecter->unregister_linger(ctx->linger_id);
+  map<uint64_t, WatchNotifyInfo *>::iterator iter = watch_notify_info.find(cookie);
+  if (iter == watch_notify_info.end())
+    return -EBADF;
 
-    watch_notify_info.erase(iter);
-    lock.Unlock();
-    ldout(cct, 10) << __func__ << " dropping reference, waiting ctx="
-                  << (void *)ctx << dendl;
-    ctx->put_wait();
-    ldout(cct, 10) << __func__ << " done ctx=" << (void *)ctx << dendl;
-    lock.Lock();
-  }
+  WatchNotifyInfo *ctx = iter->second;
+  if (poid)
+    *poid = ctx->oid;
+  if (ctx->linger_id)
+    objecter->unregister_linger(ctx->linger_id);
+
+  watch_notify_info.erase(iter);
+  lock.Unlock();
+  ldout(cct, 10) << __func__ << " dropping reference, waiting ctx="
+                << (void *)ctx << dendl;
+  ctx->put_wait();
+  ldout(cct, 10) << __func__ << " done ctx=" << (void *)ctx << dendl;
+  lock.Lock();
+  return 0;
+}
 }
 
 struct C_DoWatchNotify : public Context {

diff --git a/src/librados/RadosClient.h b/src/librados/RadosClient.h
index 2f9bb2f46fa8cc32b22fe7912065f6c085eaae4c..f9e126f328a6a5373c846dccbfcff5c4c9d1de77 100644 (file)
--- a/src/librados/RadosClient.h
+++ b/src/librados/RadosClient.h
@@ -114,7 +114,7 @@ public:
 
   void register_watch_notify_callback(librados::WatchNotifyInfo *wc,
                                      uint64_t *cookie);
-  void unregister_watch_notify_callback(uint64_t cookie);
+  int unregister_watch_notify_callback(uint64_t cookie, object_t *poid);
   void handle_watch_notify(MWatchNotify *m);
   void do_watch_notify(MWatchNotify *m);
   void do_watch_error(uint64_t cookie, int err);

diff --git a/src/librados/librados.cc b/src/librados/librados.cc
index 0663f20817b17a4271d9a80565b4131b441f9995..b396f708952252505c3d69fd61ea3d68f565f118 100644 (file)
--- a/src/librados/librados.cc
+++ b/src/librados/librados.cc
@@ -1675,9 +1675,13 @@ int librados::IoCtx::watch(const string& oid, uint64_t *cookie,
 
 int librados::IoCtx::unwatch(const string& oid, uint64_t handle)
 {
-  uint64_t cookie = handle;
   object_t obj(oid);
-  return io_ctx_impl->unwatch(obj, cookie);
+  return io_ctx_impl->unwatch(handle);
+}
+
+int librados::IoCtx::unwatch(uint64_t handle)
+{
+  return io_ctx_impl->unwatch(handle);
 }
 
 int librados::IoCtx::notify(const string& oid, uint64_t ver, bufferlist& bl)
@@ -3822,12 +3826,21 @@ extern "C" int rados_unwatch(rados_ioctx_t io, const char *o, uint64_t handle)
   tracepoint(librados, rados_unwatch_enter, io, o, handle);
   uint64_t cookie = handle;
   librados::IoCtxImpl *ctx = (librados::IoCtxImpl *)io;
-  object_t oid(o);
-  int retval = ctx->unwatch(oid, cookie);
+  int retval = ctx->unwatch(cookie);
   tracepoint(librados, rados_unwatch_exit, retval);
   return retval;
 }
 
+extern "C" int rados_unwatch2(rados_ioctx_t io, uint64_t handle)
+{
+  tracepoint(librados, rados_unwatch2_enter, io, handle);
+  uint64_t cookie = handle;
+  librados::IoCtxImpl *ctx = (librados::IoCtxImpl *)io;
+  int retval = ctx->unwatch(cookie);
+  tracepoint(librados, rados_unwatch2_exit, retval);
+  return retval;
+}
+
 extern "C" int rados_notify(rados_ioctx_t io, const char *o,
                            uint64_t ver, const char *buf, int buf_len)
 {

diff --git a/src/test/librados/watch_notify.cc b/src/test/librados/watch_notify.cc
index 774de0938a1fb472db6dfd77557fb8535e06a9d2..eb5ca096b8f94f58e2e5566bda74f89e12622a2c 100644 (file)
--- a/src/test/librados/watch_notify.cc
+++ b/src/test/librados/watch_notify.cc
@@ -266,7 +266,7 @@ TEST_F(LibRadosWatchNotify, Watch2Timeout) {
   ASSERT_EQ(0u, notify_cookies.size());
 
   // re-watch
-  rados_unwatch(ioctx, notify_oid, handle);
+  rados_unwatch2(ioctx, handle);
   handle = 0;
   ASSERT_EQ(0,
            rados_watch2(ioctx, notify_oid, &handle,
@@ -290,7 +290,7 @@ TEST_F(LibRadosWatchNotify, Watch2Timeout) {
   }
   ASSERT_EQ(1u, notify_cookies.size());
 
-  rados_unwatch(ioctx, notify_oid, handle);
+  rados_unwatch2(ioctx, handle);
 }
 
 // --
@@ -323,7 +323,7 @@ TEST_F(LibRadosWatchNotify, WatchNotify2) {
   ASSERT_EQ(1, notify_cookies.count(handle));
   ASSERT_EQ(5, reply_map.begin()->second.length());
   ASSERT_EQ(0, strncmp("reply", reply_map.begin()->second.c_str(), 5));
-  rados_unwatch(ioctx, notify_oid, handle);
+  rados_unwatch2(ioctx, handle);
 }
 
 TEST_P(LibRadosWatchNotifyPP, WatchNotify2) {
@@ -351,7 +351,7 @@ TEST_P(LibRadosWatchNotifyPP, WatchNotify2) {
   ASSERT_EQ(1u, reply_map.size());
   ASSERT_EQ(5, reply_map.begin()->second.length());
   ASSERT_EQ(0, strncmp("reply", reply_map.begin()->second.c_str(), 5));
-  ioctx.unwatch(notify_oid, handle);
+  ioctx.unwatch(handle);
 }
 
 // --
@@ -391,8 +391,8 @@ TEST_F(LibRadosWatchNotify, WatchNotify2Multi) {
   ASSERT_EQ(1, notify_cookies.count(handle1));
   ASSERT_EQ(1, notify_cookies.count(handle2));
   ASSERT_EQ(0, strncmp("reply", reply_map.begin()->second.c_str(), 5));
-  rados_unwatch(ioctx, notify_oid, handle1);
-  rados_unwatch(ioctx, notify_oid, handle2);
+  rados_unwatch2(ioctx, handle1);
+  rados_unwatch2(ioctx, handle2);
 }
 
 // --
@@ -432,7 +432,7 @@ TEST_F(LibRadosWatchNotify, WatchNotify2Timeout) {
                             &reply_buf, &reply_buf_len));
   ASSERT_EQ(1u, notify_cookies.size());
 
-  rados_unwatch(ioctx, notify_oid, handle);
+  rados_unwatch2(ioctx, handle);
 }
 
 TEST_P(LibRadosWatchNotifyPP, WatchNotify2Timeout) {
@@ -460,7 +460,7 @@ TEST_P(LibRadosWatchNotifyPP, WatchNotify2Timeout) {
   while (!notify_failed && --wait)
     sleep(1);
   ASSERT_TRUE(notify_failed);
-  ioctx.unwatch(notify_oid, handle);
+  ioctx.unwatch(handle);
 }
 
 // --

diff --git a/src/tracing/librados.tp b/src/tracing/librados.tp
index 339c2cedebb13135216b757f0240268c98db8828..49a526119a041c8093642f7be04161c729f9d1bc 100644 (file)
--- a/src/tracing/librados.tp
+++ b/src/tracing/librados.tp
@@ -2236,6 +2236,24 @@ TRACEPOINT_EVENT(librados, rados_unwatch_exit,
     )
 )
 
+TRACEPOINT_EVENT(librados, rados_unwatch2_enter,
+    TP_ARGS(
+        rados_ioctx_t, ioctx,
+        uint64_t, handle),
+    TP_FIELDS(
+        ctf_integer_hex(rados_ioctx_t, ioctx, ioctx)
+        ctf_integer(uint64_t, handle, handle)
+    )
+)
+
+TRACEPOINT_EVENT(librados, rados_unwatch2_exit,
+    TP_ARGS(
+        int, retval),
+    TP_FIELDS(
+        ctf_integer(int, retval, retval)
+    )
+)
+
 TRACEPOINT_EVENT(librados, rados_notify_enter,
     TP_ARGS(
         rados_ioctx_t, ioctx,