PGTransaction,ReplicatedPG: clarify handling of noop operations

The offending transaction was [call rbd.copyup,delete] on a non-existent
object.  PGTransaction incorrectly ended up with Create and delete_first
causing a transaction beginning with trying to collection_move_rename a
non-existent head object.  In fact, if we delete an object which the
transaction currently claims to be creating, the transaction should show
as empty (for that object).  Rather than going through the normal write
pipeline for that case, let's just record a 0 error code in the log and
call it a day.  That way, the transaction generating code only needs to
worry about about updates to actual objects.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/osd/PGTransaction.h b/src/osd/PGTransaction.h
index a8c0608cc764fd52368cdf14ba3470a64dc25a24..7ef85abaeabfc49296cebaa2cd7e48942b416693 100644 (file)
--- a/src/osd/PGTransaction.h
+++ b/src/osd/PGTransaction.h
@@ -92,6 +92,9 @@ public:
     bool is_fresh_object() const {
       return boost::get<Init::None>(&init_type) == nullptr;
     }
+    bool is_rename() const {
+      return boost::get<Init::Rename>(&init_type) != nullptr;
+    }
     bool has_source(hobject_t *source = nullptr) const {
       return match(
        init_type,
@@ -295,14 +298,19 @@ public:
     op.init_type = ObjectOperation::Init::Rename{source};
   }
 
-  /// Remove
+  /// Remove -- must not be called on rename target
   void remove(
     const hobject_t &hoid          ///< [in] obj to remove
     ) {
     auto &op = get_object_op_for_modify(hoid);
-    assert(!op.updated_snaps);
-    op = ObjectOperation();
-    op.delete_first = true;
+    if (!op.is_fresh_object()) {
+      assert(!op.updated_snaps);
+      op = ObjectOperation();
+      op.delete_first = true;
+    } else {
+      assert(!op.is_rename());
+      op_map.erase(hoid); // make it a noop if it's a fresh object
+    }
   }
 
   void update_snaps(

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index b69e90349d7afdf6b2fac97d9cf6683ec3ac2ef7..3143d4e8dc4aa6b9205fee90f61dc177721fd25f 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -3220,8 +3220,10 @@ void ReplicatedPG::execute_ctx(OpContext *ctx)
   }
 
   if (ctx->update_log_only) {
+    if (result >= 0)
+      do_osd_op_effects(ctx, m->get_connection());
+
     dout(20) << __func__ << " update_log_only -- result=" << result << dendl;
-    assert(result < 0);
     // save just what we need from ctx
     MOSDOpReply *reply = ctx->reply;
     ctx->reply = nullptr;
@@ -6737,9 +6739,13 @@ int ReplicatedPG::prepare_transaction(OpContext *ctx)
     return result;
   }
 
-  // read-op?  done?
+  // read-op?  write-op noop? done?
   if (ctx->op_t->empty() && !ctx->modify) {
     unstable_stats.add(ctx->delta_stats);
+    if (ctx->op->may_write() &&
+       get_osdmap()->test_flag(CEPH_OSDMAP_REQUIRE_KRAKEN)) {
+      ctx->update_log_only = true;
+    }
     return result;
   }