Merge pull request #671 from ceph/wip-tmap

remove tmap->omap auto-upgrade

Reviewed-by: Yehuda Sadeh <yehuda@inktank.com>

7 files changed, 99 insertions, 125 deletions

diff --git a/PendingReleaseNotes b/PendingReleaseNotes
index ef52aca884b..9a751ffdb49 100644
--- a/PendingReleaseNotes
+++ b/PendingReleaseNotes
@@ -35,3 +35,19 @@ v0.70
 v0.71
 ~~~~~
 
+* The MDS now disallows snapshots by default as they are not
+  considered stable.  The command 'ceph mds set allow_snaps' will
+  enable them.
+
+* For clusters that were created before v0.44 (pre-argonaut, Spring
+  2012) and store radosgw data, the auto-upgrade from TMAP to OMAP
+  objects has been disabled.  Before upgrading, make sure that any
+  buckets created on pre-argonaut releases have been modified (e.g.,
+  by PUTing and then DELETEing an object from each bucket).  Any
+  cluster created with argonaut (v0.48) or a later release or not
+  using radosgw never relied on the automatic conversion and is not
+  affected by this change.
+
+* Any direct users of the 'tmap' portion of the librados API should be
+  aware that the automatic tmap -> omap conversion functionality has
+  been removed.
diff --git a/qa/workunits/rados/test_tmap_to_omap.sh b/qa/workunits/rados/test_tmap_to_omap.sh
new file mode 100755
index 00000000000..76656ad726b
--- /dev/null
+++ b/qa/workunits/rados/test_tmap_to_omap.sh
@@ -0,0 +1,28 @@
+#!/bin/sh -ex
+
+expect_false()
+{
+	set -x
+	if "$@"; then return 1; else return 0; fi
+}
+
+pool="pool-$$"
+rados mkpool $pool
+
+rados -p $pool tmap set foo key1 value1
+rados -p $pool tmap set foo key2 value2
+rados -p $pool tmap set foo key2 value2
+rados -p $pool tmap dump foo | grep key1
+rados -p $pool tmap dump foo | grep key2
+rados -p $pool tmap-to-omap foo
+expect_false rados -p $pool tmap dump foo
+expect_false rados -p $pool tmap dump foo
+
+rados -p $pool listomapkeys foo | grep key1
+rados -p $pool listomapkeys foo | grep key2
+rados -p $pool getomapval foo key1 | grep value1
+rados -p $pool getomapval foo key2 | grep value2
+
+rados rmpool $pool $pool --yes-i-really-really-mean-it
+
+echo OK
diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index d7505306b6e..fad831f5543 100644
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -362,12 +362,6 @@ OPTION(mds_standby_replay, OPT_BOOL, false)
 // If true, compact leveldb store on mount
 OPTION(osd_compact_leveldb_on_mount, OPT_BOOL, false)
 
-// If true, uses tmap as initial value for omap on old objects
-OPTION(osd_auto_upgrade_tmap, OPT_BOOL, true)
-
-// If true, TMAPPUT sets uses_tmap DEBUGGING ONLY
-OPTION(osd_tmapput_sets_uses_tmap, OPT_BOOL, false)
-
 // Maximum number of backfills to or from a single osd
 OPTION(osd_max_backfills, OPT_U64, 10)
 
diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 004b5193e4b..fb5e45a1a71 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -3307,11 +3307,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  }
 	}
 
-	if (cct->_conf->osd_tmapput_sets_uses_tmap) {
-	  assert(cct->_conf->osd_auto_upgrade_tmap);
-	  oi.set_flag(object_info_t::FLAG_USES_TMAP);
-	}
-
 	// write it
 	vector<OSDOp> nops(1);
 	OSDOp& newop = nops[0];
@@ -3357,29 +3352,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	}
 	set<string> out_set;
 
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  dout(20) << "CEPH_OSD_OP_OMAPGETKEYS: "
-		   << " Reading " << oi.soid << " omap from tmap" << dendl;
-	  map<string, bufferlist> vals;
-	  bufferlist header;
-	  int r = _get_tmap(ctx, &vals, &header);
-	  if (r == 0) {
-	    map<string, bufferlist>::iterator iter =
-	      vals.upper_bound(start_after);
-	    for (uint64_t i = 0;
-		 i < max_return && iter != vals.end();
-		 ++i, iter++) {
-	      out_set.insert(iter->first);
-	    }
-	    ::encode(out_set, osd_op.outdata);
-	    ctx->delta_stats.num_rd_kb += SHIFT_ROUND_UP(osd_op.outdata.length(), 10);
-	    ctx->delta_stats.num_rd++;
-	    break;
-	  }
-	  dout(10) << "failed, reading from omap" << dendl;
-	  // No valid tmap, use omap
-	}
-
 	{
 	  ObjectMap::ObjectMapIterator iter = osd->store->get_omap_iterator(
 	    coll, soid
@@ -3415,30 +3387,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	}
 	map<string, bufferlist> out_set;
 
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  dout(20) << "CEPH_OSD_OP_OMAPGETVALS: "
-		   << " Reading " << oi.soid << " omap from tmap" << dendl;
-	  map<string, bufferlist> vals;
-	  bufferlist header;
-	  int r = _get_tmap(ctx, &vals, &header);
-	  if (r == 0) {
-	    map<string, bufferlist>::iterator iter = vals.upper_bound(start_after);
-	    if (filter_prefix > start_after) iter = vals.lower_bound(filter_prefix);
-	    for (uint64_t i = 0;
-		 i < max_return && iter != vals.end() &&
-		   iter->first.substr(0, filter_prefix.size()) == filter_prefix;
-		 ++i, iter++) {
-	      out_set.insert(*iter);
-	    }
-	    ::encode(out_set, osd_op.outdata);
-	    ctx->delta_stats.num_rd_kb += SHIFT_ROUND_UP(osd_op.outdata.length(), 10);
-	    ctx->delta_stats.num_rd++;
-	    break;
-	  }
-	  // No valid tmap, use omap
-	  dout(10) << "failed, reading from omap" << dendl;
-	}
-
 	{
 	  ObjectMap::ObjectMapIterator iter = osd->store->get_omap_iterator(
 	    coll, soid
@@ -3466,19 +3414,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
     case CEPH_OSD_OP_OMAPGETHEADER:
       ++ctx->num_read;
       {
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  dout(20) << "CEPH_OSD_OP_OMAPGETHEADER: "
-		   << " Reading " << oi.soid << " omap from tmap" << dendl;
-	  map<string, bufferlist> vals;
-	  bufferlist header;
-	  int r = _get_tmap(ctx, &vals, &header);
-	  if (r == 0) {
-	    osd_op.outdata.claim(header);
-	    break;
-	  }
-	  // No valid tmap, fall through to omap
-	  dout(10) << "failed, reading from omap" << dendl;
-	}
 	osd->store->omap_get_header(coll, soid, &osd_op.outdata);
 	ctx->delta_stats.num_rd_kb += SHIFT_ROUND_UP(osd_op.outdata.length(), 10);
 	ctx->delta_stats.num_rd++;
@@ -3497,28 +3432,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  goto fail;
 	}
 	map<string, bufferlist> out;
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  dout(20) << "CEPH_OSD_OP_OMAPGET: "
-		   << " Reading " << oi.soid << " omap from tmap" << dendl;
-	  map<string, bufferlist> vals;
-	  bufferlist header;
-	  int r = _get_tmap(ctx, &vals, &header);
-	  if (r == 0) {
-	    for (set<string>::iterator iter = keys_to_get.begin();
-		 iter != keys_to_get.end();
-		 ++iter) {
-	      if (vals.count(*iter)) {
-		out.insert(*(vals.find(*iter)));
-	      }
-	    }
-	    ::encode(out, osd_op.outdata);
-	    ctx->delta_stats.num_rd_kb += SHIFT_ROUND_UP(osd_op.outdata.length(), 10);
-	    ctx->delta_stats.num_rd++;
-	    break;
-	  }
-	  // No valid tmap, use omap
-	  dout(10) << "failed, reading from omap" << dendl;
-	}
 	osd->store->omap_get_values(coll, soid, keys_to_get, &out);
 	::encode(out, osd_op.outdata);
 	ctx->delta_stats.num_rd_kb += SHIFT_ROUND_UP(osd_op.outdata.length(), 10);
@@ -3596,9 +3509,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
     case CEPH_OSD_OP_OMAPSETVALS:
       ++ctx->num_write;
       {
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  _copy_up_tmap(ctx);
-	}
 	if (!obs.exists) {
 	  ctx->delta_stats.num_objects++;
 	  obs.exists = true;
@@ -3626,9 +3536,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
     case CEPH_OSD_OP_OMAPSETHEADER:
       ++ctx->num_write;
       {
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  _copy_up_tmap(ctx);
-	}
 	if (!obs.exists) {
 	  ctx->delta_stats.num_objects++;
 	  obs.exists = true;
@@ -3646,9 +3553,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  result = -ENOENT;
 	  break;
 	}
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  _copy_up_tmap(ctx);
-	}
 	t.touch(coll, soid);
 	t.omap_clear(coll, soid);
 	ctx->delta_stats.num_wr++;
@@ -3662,9 +3566,6 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  result = -ENOENT;
 	  break;
 	}
-	if (oi.test_flag(object_info_t::FLAG_USES_TMAP) && cct->_conf->osd_auto_upgrade_tmap) {
-	  _copy_up_tmap(ctx);
-	}
 	t.touch(coll, soid);
 	set<string> to_rm;
 	try {
@@ -3838,22 +3739,6 @@ int ReplicatedPG::_get_tmap(OpContext *ctx,
   return 0;
 }
 
-int ReplicatedPG::_copy_up_tmap(OpContext *ctx)
-{
-  dout(20) << "copying up tmap for " << ctx->new_obs.oi.soid << dendl;
-  ctx->new_obs.oi.clear_flag(object_info_t::FLAG_USES_TMAP);
-  map<string, bufferlist> vals;
-  bufferlist header;
-  int r = _get_tmap(ctx, &vals, &header);
-  if (r < 0)
-    return 0;
-  ctx->op_t.omap_setkeys(coll, ctx->new_obs.oi.soid,
-			 vals);
-  ctx->op_t.omap_setheader(coll, ctx->new_obs.oi.soid,
-			   header);
-  return 0;
-}
-
 inline int ReplicatedPG::_delete_head(OpContext *ctx)
 {
   SnapSet& snapset = ctx->new_snapset;
diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index 4b91a2135dc..05edcef6adf 100644
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -856,7 +856,6 @@ private:
 
   int _get_tmap(OpContext *ctx, map<string, bufferlist> *out,
 		bufferlist *header);
-  int _copy_up_tmap(OpContext *ctx);
   int _delete_head(OpContext *ctx);
   int _rollback_to(OpContext *ctx, ceph_osd_op& op);
 public:
diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 72ee32c4cc5..59b71cc6f67 100644
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -2101,7 +2101,7 @@ struct object_info_t {
     FLAG_WHITEOUT = 1<<1,  // object logically does not exist
     FLAG_DIRTY    = 1<<2,  // object has been modified since last flushed or undirtied
     // ...
-    FLAG_USES_TMAP = 1<<8,
+    FLAG_USES_TMAP = 1<<8,  // deprecated; no longer used.
   } flag_t;
 
   flag_t flags;
diff --git a/src/tools/rados/rados.cc b/src/tools/rados/rados.cc
index fdf7e61d41e..ad8eaa3e1a4 100644
--- a/src/tools/rados/rados.cc
+++ b/src/tools/rados/rados.cc
@@ -98,6 +98,7 @@ void usage(ostream& out)
 "   rmomapkey <obj-name> <key>\n"
 "   getomapheader <obj-name>\n"
 "   setomapheader <obj-name> <val>\n"
+"   tmap-to-omap <obj-name>          convert tmap keys/values to omap\n"
 "   listwatchers <obj-name>          list the watchers of this object\n"
 "\n"
 "IMPORT AND EXPORT\n"
@@ -1813,8 +1814,15 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
       bufferlist::iterator p = outdata.begin();
       bufferlist header;
       map<string, bufferlist> kv;
-      ::decode(header, p);
-      ::decode(kv, p);
+      try {
+	::decode(header, p);
+	::decode(kv, p);
+      }
+      catch (buffer::error& e) {
+	cerr << "error decoding tmap " << pool_name << "/" << oid << std::endl;
+	ret = -EINVAL;
+	goto out;
+      }
       cout << "header (" << header.length() << " bytes):\n";
       header.hexdump(cout);
       cout << "\n";
@@ -1841,6 +1849,50 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
     }
   }
 
+  else if (strcmp(nargs[0], "tmap-to-omap") == 0) {
+    if (!pool_name || nargs.size() < 2)
+      usage_exit();
+    string oid(nargs[1]);
+
+    bufferlist bl;
+    int r = io_ctx.tmap_get(oid, bl);
+    if (r < 0) {
+      ret = r;
+      cerr << "error reading tmap " << pool_name << "/" << oid
+	   << ": " << cpp_strerror(ret) << std::endl;
+      goto out;
+    }
+    bufferlist hdr;
+    map<string, bufferlist> kv;
+    bufferlist::iterator p = bl.begin();
+    try {
+      ::decode(hdr, p);
+      ::decode(kv, p);
+    }
+    catch (buffer::error& e) {
+      cerr << "error decoding tmap " << pool_name << "/" << oid << std::endl;
+      ret = -EINVAL;
+      goto out;
+    }
+    if (!p.end()) {
+      cerr << "error decoding tmap (stray trailing data) in " << pool_name << "/" << oid << std::endl;
+      ret = -EINVAL;
+      goto out;
+    }
+    librados::ObjectWriteOperation wr;
+    wr.omap_set_header(hdr);
+    wr.omap_set(kv);
+    wr.truncate(0);  // delete the old tmap data
+    r = io_ctx.operate(oid, &wr);
+    if (r < 0) {
+      ret = r;
+      cerr << "error writing tmap data as omap on " << pool_name << "/" << oid
+	   << ": " << cpp_strerror(ret) << std::endl;
+      goto out;
+    }
+    ret = 0;
+  }
+
   else if (strcmp(nargs[0], "mkpool") == 0) {
     int auid = 0;
     __u8 crush_rule = 0;