From: Ilya Dryomov <idryomov@gmail.com>
Date: Thu, 14 Feb 2019 20:27:12 +0000 (+0100)
Subject: librbd: send copyup and hint only once
X-Git-Tag: v14.1.0~34^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=34d6374c1959076168614864e29b980b01a241ac;p=ceph.git

librbd: send copyup and hint only once

In the deep-copy case, don't send copyup and hint twice (once with the
blank and once with the current snapshot context).

Preserve the workaround for compare-and-write added in commit
f6db9b8027b6 ("librbd: copyup state machine needs to handle empty write
ops").

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
index 7168cafda07f..7e34112c7414 100644
--- a/src/librbd/io/CopyupRequest.cc
+++ b/src/librbd/io/CopyupRequest.cc
@@ -118,12 +118,6 @@ void CopyupRequest<I>::complete_requests(int r) {
 
 template <typename I>
 bool CopyupRequest<I>::send_copyup() {
-  bool copy_on_read = m_pending_requests.empty();
-  bool add_copyup_op = !m_copyup_data.is_zero();
-  if (!add_copyup_op) {
-    m_copyup_data.clear();
-  }
-
   ldout(m_ictx->cct, 20) << "oid " << m_oid << dendl;
   m_state = STATE_COPYUP;
 
@@ -133,14 +127,17 @@ bool CopyupRequest<I>::send_copyup() {
 
   std::vector<librados::snap_t> snaps;
 
+  bool copy_on_read = m_pending_requests.empty();
+  bool deep_copyup = !snapc.snaps.empty() && !m_copyup_data.is_zero();
+  if (m_copyup_data.is_zero()) {
+    m_copyup_data.clear();
+  }
+
   Mutex::Locker locker(m_lock);
   int r;
-  if (copy_on_read || (!snapc.snaps.empty() && add_copyup_op)) {
-
+  if (copy_on_read || deep_copyup) {
     librados::ObjectWriteOperation copyup_op;
     copyup_op.exec("rbd", "copyup", m_copyup_data);
-    m_copyup_data.clear();
-
     ObjectRequest<I>::add_write_hint(*m_ictx, &copyup_op);
 
     // send only the copyup request with a blank snapshot context so that
@@ -162,16 +159,29 @@ bool CopyupRequest<I>::send_copyup() {
 
   if (!copy_on_read) {
     librados::ObjectWriteOperation write_op;
-    write_op.exec("rbd", "copyup", m_copyup_data);
+    if (!deep_copyup) {
+      write_op.exec("rbd", "copyup", m_copyup_data);
+      ObjectRequest<I>::add_write_hint(*m_ictx, &write_op);
+    }
 
     // merge all pending write ops into this single RADOS op
-    ObjectRequest<I>::add_write_hint(*m_ictx, &write_op);
     for (auto req : m_pending_requests) {
       ldout(m_ictx->cct, 20) << "add_copyup_ops " << req << dendl;
       req->add_copyup_ops(&write_op);
     }
 
+    // compare-and-write doesn't add any write ops (copyup+cmpext+write
+    // can't be executed in the same RADOS op because, unless the object
+    // was already present in the clone, cmpext wouldn't see it)
+    if (!write_op.size()) {
+      return false;
+    }
+
     m_pending_copyups++;
+    ldout(m_ictx->cct, 20) << (!deep_copyup && write_op.size() > 2 ?
+                               "copyup + ops" : !deep_copyup ?
+                                                "copyup" : "ops")
+                           << " with current snapshot context" << dendl;
 
     snaps.insert(snaps.end(), snapc.snaps.begin(), snapc.snaps.end());
     librados::AioCompletion *comp = util::create_rados_callback(this);