librbd: batch ObjectMap updations upon trim

Shrinking an image can result in huge number of ObjectMap
updates -- two for each object being removed. This results
in lots of requests to be send to OSDs especially when
trimming a gigantus image. This situation can be optimized
by sending batch ObjectMap updates, significantly cutting
down the OSD traffic.

Fixes: http://tracker.ceph.com/issues/17356
Signed-off-by: Venky Shankar <vshankar@redhat.com>

src/librbd/AioObjectRequest.h

@@ -348,10 +348,6 @@ class AioObjectTrim : public AbstractAioObjectWrite {
   virtual void pre_object_map_update(uint8_t *new_state) {
     *new_state = OBJECT_PENDING;
   }
-
-  virtual bool post_object_map_update() {
-    return true;
-  }
 };
 
 class AioObjectTruncate : public AbstractAioObjectWrite {

src/librbd/operation/TrimRequest.cc

@@ -131,8 +131,18 @@ bool TrimRequest<I>::should_complete(int r)
 
   RWLock::RLocker owner_lock(image_ctx.owner_lock);
   switch (m_state) {
+  case STATE_PRE_COPYUP:
+    ldout(cct, 5) << " PRE_COPYUP" << dendl;
+    send_copyup_objects();
+    break;
+
   case STATE_COPYUP_OBJECTS:
     ldout(cct, 5) << " COPYUP_OBJECTS" << dendl;
+    send_post_copyup();
+    break;
+
+  case STATE_POST_COPYUP:
+    ldout(cct, 5) << " POST_COPYUP" << dendl;
     send_pre_remove();
     break;
 
@@ -170,11 +180,11 @@ bool TrimRequest<I>::should_complete(int r)
 
 template <typename I>
 void TrimRequest<I>::send() {
-  send_copyup_objects();
+  send_update_objectmap();
 }
 
 template <typename I>
-void TrimRequest<I>::send_copyup_objects() {
+void TrimRequest<I>::send_update_objectmap() {
   I &image_ctx = this->m_image_ctx;
   assert(image_ctx.owner_lock.is_locked());
   assert(image_ctx.exclusive_lock == nullptr ||
@@ -185,43 +195,51 @@ void TrimRequest<I>::send_copyup_objects() {
     return;
   }
 
-  ::SnapContext snapc;
   bool has_snapshots;
   uint64_t parent_overlap;
   {
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
     RWLock::RLocker parent_locker(image_ctx.parent_lock);
 
-    snapc = image_ctx.snapc;
     has_snapshots = !image_ctx.snaps.empty();
     int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &parent_overlap);
     assert(r == 0);
   }
 
   // copyup is only required for portion of image that overlaps parent
-  uint64_t copyup_end = Striper::get_num_objects(image_ctx.layout,
-                                                 parent_overlap);
+  m_copyup_end = Striper::get_num_objects(image_ctx.layout, parent_overlap);
   // TODO: protect against concurrent shrink and snap create?
-  if (copyup_end <= m_delete_start || !has_snapshots) {
+  if (m_copyup_end <= m_delete_start || !has_snapshots) {
     send_pre_remove();
-    return;
+  } else {
+    send_pre_copyup();
   }
+}
 
-  uint64_t copyup_start = m_delete_start;
-  m_delete_start = copyup_end;
+template<typename I>
+void TrimRequest<I>::send_copyup_objects() {
+  I &image_ctx = this->m_image_ctx;
+  assert(image_ctx.owner_lock.is_locked());
 
   ldout(image_ctx.cct, 5) << this << " send_copyup_objects: "
-			    << " start object=" << copyup_start << ", "
-			    << " end object=" << copyup_end << dendl;
+                          << " start object=" << m_copyup_start << ", "
+                          << " end object=" << m_copyup_end << dendl;
   m_state = STATE_COPYUP_OBJECTS;
 
+  ::SnapContext snapc;
+  {
+    RWLock::RLocker snap_locker(image_ctx.snap_lock);
+    RWLock::RLocker parent_locker(image_ctx.parent_lock);
+    snapc = image_ctx.snapc;
+  }
+
   Context *ctx = this->create_callback_context();
   typename AsyncObjectThrottle<I>::ContextFactory context_factory(
     boost::lambda::bind(boost::lambda::new_ptr<C_CopyupObject<I> >(),
       boost::lambda::_1, &image_ctx, snapc, boost::lambda::_2));
   AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
-    this, image_ctx, context_factory, ctx, &m_prog_ctx, copyup_start,
-    copyup_end);
+    this, image_ctx, context_factory, ctx, &m_prog_ctx, m_copyup_start,
+    m_copyup_end);
   throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 
@@ -245,6 +263,46 @@ void TrimRequest<I>::send_remove_objects() {
   throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 
+template<typename I>
+void TrimRequest<I>::send_pre_copyup() {
+  I &image_ctx = this->m_image_ctx;
+  assert(image_ctx.owner_lock.is_locked());
+  if (m_delete_start >= m_num_objects) {
+    send_clean_boundary();
+    return;
+  }
+
+  m_copyup_start = m_delete_start;
+  m_delete_start = m_copyup_end;
+
+  bool copyup_objects = false;
+  {
+    RWLock::RLocker snap_locker(image_ctx.snap_lock);
+    if (image_ctx.object_map == nullptr) {
+      copyup_objects = true;
+    } else {
+      ldout(image_ctx.cct, 5) << this << " send_pre_copyup: "
+                              << " copyup_start=" << m_copyup_start
+                              << " copyup_end=" << m_copyup_end << dendl;
+      m_state = STATE_PRE_COPYUP;
+
+      assert(image_ctx.exclusive_lock->is_lock_owner());
+
+      Context *ctx = this->create_callback_context();
+      RWLock::WLocker object_map_locker(image_ctx.object_map_lock);
+      if (!image_ctx.object_map->aio_update(m_copyup_start, m_copyup_end,
+                                            OBJECT_PENDING, OBJECT_EXISTS, ctx)) {
+        delete ctx;
+        copyup_objects = true;
+      }
+    }
+  }
+
+  if (copyup_objects) {
+    send_copyup_objects();
+  }
+}
+
 template <typename I>
 void TrimRequest<I>::send_pre_remove() {
   I &image_ctx = this->m_image_ctx;
@@ -286,6 +344,39 @@ void TrimRequest<I>::send_pre_remove() {
   }
 }
 
+template<typename I>
+void TrimRequest<I>::send_post_copyup() {
+  I &image_ctx = this->m_image_ctx;
+  assert(image_ctx.owner_lock.is_locked());
+
+  bool pre_remove_objects = false;
+  {
+    RWLock::RLocker snap_locker(image_ctx.snap_lock);
+    if (image_ctx.object_map == nullptr) {
+      pre_remove_objects = true;
+    } else {
+      ldout(image_ctx.cct, 5) << this << " send_post_copyup:"
+                              << " copyup_start=" << m_copyup_start
+                              << " copyup_end=" << m_copyup_end << dendl;
+      m_state = STATE_POST_COPYUP;
+
+      assert(image_ctx.exclusive_lock->is_lock_owner());
+
+      Context *ctx = this->create_callback_context();
+      RWLock::WLocker object_map_locker(image_ctx.object_map_lock);
+      if (!image_ctx.object_map->aio_update(m_copyup_start, m_copyup_end,
+                                            OBJECT_NONEXISTENT, OBJECT_PENDING, ctx)) {
+        delete ctx;
+        pre_remove_objects = true;
+      }
+    }
+  }
+
+  if (pre_remove_objects) {
+    send_pre_remove();
+  }
+}
+
 template <typename I>
 void TrimRequest<I>::send_post_remove() {
   I &image_ctx = this->m_image_ctx;

src/librbd/operation/TrimRequest.h

@@ -64,7 +64,9 @@ class TrimRequest : public AsyncRequest<ImageCtxT>
    */ 
 
   enum State {
+    STATE_PRE_COPYUP,
     STATE_COPYUP_OBJECTS,
+    STATE_POST_COPYUP,
     STATE_PRE_REMOVE,
     STATE_REMOVE_OBJECTS,
     STATE_POST_REMOVE,
@@ -83,13 +85,19 @@ class TrimRequest : public AsyncRequest<ImageCtxT>
   uint64_t m_new_size;
   ProgressContext &m_prog_ctx;
 
+  uint64_t m_copyup_start;
+  uint64_t m_copyup_end;
+
   TrimRequest(ImageCtxT &image_ctx, Context *on_finish,
 	      uint64_t original_size, uint64_t new_size,
 	      ProgressContext &prog_ctx);
 
+  void send_update_objectmap();
   void send_copyup_objects();
   void send_remove_objects();
+  void send_pre_copyup();
   void send_pre_remove();
+  void send_post_copyup();
   void send_post_remove();
   void send_clean_boundary();
   void send_finish(int r);