osd: change omap data encoding in object_copy_data_t

Pass the omap data in the struct as an opaque (encoded) bufferlist.  This
avoids a decode into an STL map, simplifying the crc calculation.  The
win isn't huge (yet) because we still turn it into a map to pass it down
to the ObjectStore method, but that too will change in time.

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

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index ca795d10507d42baff105cc7a13e08e2e20a1d81..9a6b90884939e6728b581816260c85d62dee8fb9 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -5834,10 +5834,11 @@ struct C_Copyfrom : public Context {
 struct C_CopyFrom_AsyncReadCb : public Context {
   OSDOp *osd_op;
   object_copy_data_t reply_obj;
+  uint64_t features;
   bool classic;
   size_t len;
-  C_CopyFrom_AsyncReadCb(OSDOp *osd_op, bool classic) :
-    osd_op(osd_op), classic(classic), len(0) {}
+  C_CopyFrom_AsyncReadCb(OSDOp *osd_op, uint64_t features, bool classic) :
+    osd_op(osd_op), features(features), classic(classic), len(0) {}
   void finish(int r) {
     assert(len > 0);
     assert(len <= reply_obj.data.length());
@@ -5847,7 +5848,7 @@ struct C_CopyFrom_AsyncReadCb : public Context {
     if (classic) {
       reply_obj.encode_classic(osd_op->outdata);
     } else {
-      ::encode(reply_obj, osd_op->outdata);
+      ::encode(reply_obj, osd_op->outdata, features);
     }
   }
 };
@@ -5873,11 +5874,13 @@ int ReplicatedPG::fill_in_copy_get(
     return result;
   }
 
+  uint64_t features = ctx->op->get_req()->get_connection()->get_features();
+
   bool async_read_started = false;
   object_copy_data_t _reply_obj;
   C_CopyFrom_AsyncReadCb *cb = NULL;
   if (pool.info.require_rollback()) {
-    cb = new C_CopyFrom_AsyncReadCb(&osd_op, classic);
+    cb = new C_CopyFrom_AsyncReadCb(&osd_op, features, classic);
   }
   object_copy_data_t &reply_obj = cb ? cb->reply_obj : _reply_obj;
   // size, mtime
@@ -5947,7 +5950,7 @@ int ReplicatedPG::fill_in_copy_get(
   }
 
   // omap
-  std::map<std::string,bufferlist>& out_omap = reply_obj.omap;
+  uint32_t omap_keys = 0;
   if (pool.info.require_rollback()) {
     cursor.omap_complete = true;
   } else {
@@ -5956,16 +5959,23 @@ int ReplicatedPG::fill_in_copy_get(
       if (cursor.omap_offset.empty()) {
        osd->store->omap_get_header(coll, oi.soid, &reply_obj.omap_header);
       }
+      bufferlist omap_data;
       ObjectMap::ObjectMapIterator iter =
        osd->store->get_omap_iterator(coll, oi.soid);
       assert(iter);
       iter->upper_bound(cursor.omap_offset);
       for (; iter->valid(); iter->next()) {
-       out_omap.insert(make_pair(iter->key(), iter->value()));
+       ++omap_keys;
+       ::encode(iter->key(), omap_data);
+       ::encode(iter->value(), omap_data);
        left -= iter->key().length() + 4 + iter->value().length() + 4;
        if (left <= 0)
          break;
       }
+      if (omap_keys) {
+       ::encode(omap_keys, reply_obj.omap_data);
+       reply_obj.omap_data.claim_append(omap_data);
+      }
       if (iter->valid()) {
        cursor.omap_offset = iter->key();
       } else {
@@ -5979,14 +5989,15 @@ int ReplicatedPG::fill_in_copy_get(
           << " " << out_attrs.size() << " attrs"
           << " " << bl.length() << " bytes"
           << " " << reply_obj.omap_header.length() << " omap header bytes"
-          << " " << out_omap.size() << " keys"
+          << " " << reply_obj.omap_data.length() << " omap data bytes in "
+          << omap_keys << " keys"
           << dendl;
   reply_obj.cursor = cursor;
   if (!async_read_started) {
     if (classic) {
       reply_obj.encode_classic(osd_op.outdata);
     } else {
-      ::encode(reply_obj, osd_op.outdata);
+      ::encode(reply_obj, osd_op.outdata, features);
     }
   }
   if (cb && !async_read_started) {
@@ -6064,7 +6075,7 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop)
   }
   op.copy_get(&cop->cursor, get_copy_chunk_size(),
              &cop->results.object_size, &cop->results.mtime,
-             &cop->attrs, &cop->data, &cop->omap_header, &cop->omap,
+             &cop->attrs, &cop->data, &cop->omap_header, &cop->omap_data,
              &cop->results.snaps, &cop->results.snap_seq,
              &cop->results.flags,
              &cop->results.source_data_digest,
@@ -6103,10 +6114,10 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, ceph_tid_t tid, int r)
     return;
   }
 
-  if (cop->omap.size())
+  if (cop->omap_data.length())
     cop->results.has_omap = true;
 
-  if (r >= 0 && pool.info.require_rollback() && cop->omap.size()) {
+  if (r >= 0 && pool.info.require_rollback() && cop->omap_data.length()) {
     r = -EOPNOTSUPP;
   }
   cop->objecter_tid = 0;
@@ -6225,7 +6236,8 @@ void ReplicatedPG::_write_copy_chunk(CopyOpRef cop, PGBackend::PGTransaction *t)
   dout(20) << __func__ << " " << cop
           << " " << cop->attrs.size() << " attrs"
           << " " << cop->data.length() << " bytes"
-          << " " << cop->omap.size() << " keys"
+          << " " << cop->omap_header.length() << " omap header bytes"
+          << " " << cop->omap_data.length() << " omap data bytes"
           << dendl;
   if (!cop->temp_cursor.attr_complete) {
     t->touch(cop->results.temp_oid);
@@ -6280,22 +6292,22 @@ void ReplicatedPG::_write_copy_chunk(CopyOpRef cop, PGBackend::PGTransaction *t)
          cop->omap_header);
        cop->omap_header.clear();
       }
-      if (cop->omap.size()) {
-       for (map<string,bufferlist>::iterator p = cop->omap.begin();
-            p != cop->omap.end(); ++p) {
-         cop->results.omap_digest = ceph_crc32c(
-           cop->results.omap_digest,
-           (const unsigned char *)p->first.data(),
-           p->first.length());
-         cop->results.omap_digest = p->second.crc32c(cop->results.omap_digest);
-       }
+      if (cop->omap_data.length()) {
+       // don't checksum the key count prefix
+       bufferlist keys;
+       keys.substr_of(cop->omap_data, 4, cop->omap_data.length() - 4);
+       cop->results.omap_digest = keys.crc32c(cop->results.omap_digest);
+
+       map<string,bufferlist> omap;
+       bufferlist::iterator p = cop->omap_data.begin();
+       ::decode(omap, p);
+       t->omap_setkeys(cop->results.temp_oid, omap);
+       cop->omap_data.clear();
       }
-      t->omap_setkeys(cop->results.temp_oid, cop->omap);
-      cop->omap.clear();
     }
   } else {
     assert(cop->omap_header.length() == 0);
-    assert(cop->omap.empty());
+    assert(cop->omap_data.length() == 0);
   }
   cop->temp_cursor = cop->cursor;
 }

diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index 57a5220cac6113e5f9c66c4d5a977f63adfa29a0..65f7773e5926d11794fd2768500c0c0ff4568d22 100644 (file)
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -160,7 +160,7 @@ public:
     map<string,bufferlist> attrs;
     bufferlist data;
     bufferlist omap_header;
-    map<string,bufferlist> omap;
+    bufferlist omap_data;
     int rval;
 
     object_copy_cursor_t temp_cursor;

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index ac6e893abfdbcf5b794ecc6f11c13451e639e34c..d12c6d475da45ae41394f7eaeba9e36cf00b9761 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -3318,7 +3318,7 @@ void object_copy_data_t::encode_classic(bufferlist& bl) const
   ::encode(mtime, bl);
   ::encode(attrs, bl);
   ::encode(data, bl);
-  ::encode(omap, bl);
+  bl.append(omap_data);
   ::encode(cursor, bl);
 }
 
@@ -3328,21 +3328,44 @@ void object_copy_data_t::decode_classic(bufferlist::iterator& bl)
   ::decode(mtime, bl);
   ::decode(attrs, bl);
   ::decode(data, bl);
-  ::decode(omap, bl);
+  {
+    map<string,bufferlist> omap;
+    ::decode(omap, bl);
+    omap_data.clear();
+    ::encode(omap, omap_data);
+  }
   ::decode(cursor, bl);
   flags = 0;
   data_digest = omap_digest = 0;
 }
 
-void object_copy_data_t::encode(bufferlist& bl) const
+void object_copy_data_t::encode(bufferlist& bl, uint64_t features) const
 {
-  ENCODE_START(4, 1, bl);
+  if ((features & CEPH_FEATURE_OSD_OBJECT_DIGEST) == 0) {
+    ENCODE_START(4, 1, bl);
+    ::encode(size, bl);
+    ::encode(mtime, bl);
+    ::encode((__u32)0, bl);  // was category; no longer used
+    ::encode(attrs, bl);
+    ::encode(data, bl);
+    bl.append(omap_data);
+    ::encode(cursor, bl);
+    ::encode(omap_header, bl);
+    ::encode(snaps, bl);
+    ::encode(snap_seq, bl);
+    ::encode(flags, bl);
+    ::encode(data_digest, bl);
+    ::encode(omap_digest, bl);
+    ENCODE_FINISH(bl);
+    return;
+  }
+
+  ENCODE_START(5, 5, bl);
   ::encode(size, bl);
   ::encode(mtime, bl);
-  ::encode((__u32)0, bl);  // was category; no longer used
   ::encode(attrs, bl);
   ::encode(data, bl);
-  ::encode(omap, bl);
+  ::encode(omap_data, bl);
   ::encode(cursor, bl);
   ::encode(omap_header, bl);
   ::encode(snaps, bl);
@@ -3355,30 +3378,60 @@ void object_copy_data_t::encode(bufferlist& bl) const
 
 void object_copy_data_t::decode(bufferlist::iterator& bl)
 {
-  DECODE_START(4, bl);
-  ::decode(size, bl);
-  ::decode(mtime, bl);
-  {
-    string category;
-    ::decode(category, bl);  // no longer used
-  }
-  ::decode(attrs, bl);
-  ::decode(data, bl);
-  ::decode(omap, bl);
-  ::decode(cursor, bl);
-  if (struct_v >= 2)
-    ::decode(omap_header, bl);
-  if (struct_v >= 3) {
-    ::decode(snaps, bl);
-    ::decode(snap_seq, bl);
+  DECODE_START(5, bl);
+  if (struct_v < 5) {
+    // old
+    ::decode(size, bl);
+    ::decode(mtime, bl);
+    {
+      string category;
+      ::decode(category, bl);  // no longer used
+    }
+    ::decode(attrs, bl);
+    ::decode(data, bl);
+    {
+      map<string,bufferlist> omap;
+      ::decode(omap, bl);
+      omap_data.clear();
+      ::encode(omap, omap_data);
+    }
+    ::decode(cursor, bl);
+    if (struct_v >= 2)
+      ::decode(omap_header, bl);
+    if (struct_v >= 3) {
+      ::decode(snaps, bl);
+      ::decode(snap_seq, bl);
+    } else {
+      snaps.clear();
+      snap_seq = 0;
+    }
+    if (struct_v >= 4) {
+      ::decode(flags, bl);
+      ::decode(data_digest, bl);
+      ::decode(omap_digest, bl);
+    }
   } else {
-    snaps.clear();
-    snap_seq = 0;
-  }
-  if (struct_v >= 4) {
-    ::decode(flags, bl);
-    ::decode(data_digest, bl);
-    ::decode(omap_digest, bl);
+    // current
+    ::decode(size, bl);
+    ::decode(mtime, bl);
+    ::decode(attrs, bl);
+    ::decode(data, bl);
+    ::decode(omap_data, bl);
+    ::decode(cursor, bl);
+    if (struct_v >= 2)
+      ::decode(omap_header, bl);
+    if (struct_v >= 3) {
+      ::decode(snaps, bl);
+      ::decode(snap_seq, bl);
+    } else {
+      snaps.clear();
+      snap_seq = 0;
+    }
+    if (struct_v >= 4) {
+      ::decode(flags, bl);
+      ::decode(data_digest, bl);
+      ::decode(omap_digest, bl);
+    }
   }
   DECODE_FINISH(bl);
 }
@@ -3405,7 +3458,9 @@ void object_copy_data_t::generate_test_instances(list<object_copy_data_t*>& o)
   bufferptr bp2("not", 3);
   bufferlist bl2;
   bl2.push_back(bp2);
-  o.back()->omap["why"] = bl2;
+  map<string,bufferlist> omap;
+  omap["why"] = bl2;
+  ::encode(omap, o.back()->omap_data);
   bufferptr databp("iamsomedatatocontain", 20);
   o.back()->data.push_back(databp);
   o.back()->omap_header.append("this is an omap header");
@@ -3425,7 +3480,7 @@ void object_copy_data_t::dump(Formatter *f) const
   f->dump_int("flags", flags);
   f->dump_unsigned("data_digest", data_digest);
   f->dump_unsigned("omap_digest", omap_digest);
-  f->dump_int("omap_size", omap.size());
+  f->dump_int("omap_data_length", omap_data.length());
   f->dump_int("omap_header_length", omap_header.length());
   f->dump_int("data_length", data.length());
   f->open_array_section("snaps");

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index ad4d5d292de5f66fd84171f4bceb1a035d38622e..c893ac526d80e8610ecc7ce3aa05e3dd55bd5400 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -2502,7 +2502,7 @@ struct object_copy_data_t {
   map<string, bufferlist> attrs;
   bufferlist data;
   bufferlist omap_header;
-  map<string, bufferlist> omap;
+  bufferlist omap_data;
 
   /// which snaps we are defined for (if a snap and not the head)
   vector<snapid_t> snaps;
@@ -2515,11 +2515,11 @@ public:
   static void generate_test_instances(list<object_copy_data_t*>& o);
   void encode_classic(bufferlist& bl) const;
   void decode_classic(bufferlist::iterator& bl);
-  void encode(bufferlist& bl) const;
+  void encode(bufferlist& bl, uint64_t features) const;
   void decode(bufferlist::iterator& bl);
   void dump(Formatter *f) const;
 };
-WRITE_CLASS_ENCODER(object_copy_data_t)
+WRITE_CLASS_ENCODER_FEATURES(object_copy_data_t)
 
 /**
  * pg creation info

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 2d7527668728d2bd0da88da733dd88c5ed47ce75..3348dbb9a06f9bc6100053c58f23eeadd4f44208 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -619,8 +619,7 @@ struct ObjectOperation {
     uint64_t *out_size;
     utime_t *out_mtime;
     std::map<std::string,bufferlist> *out_attrs;
-    bufferlist *out_data, *out_omap_header;
-    std::map<std::string,bufferlist> *out_omap;
+    bufferlist *out_data, *out_omap_header, *out_omap_data;
     vector<snapid_t> *out_snaps;
     snapid_t *out_snap_seq;
     uint32_t *out_flags;
@@ -632,7 +631,7 @@ struct ObjectOperation {
                              utime_t *m,
                              std::map<std::string,bufferlist> *a,
                              bufferlist *d, bufferlist *oh,
-                             std::map<std::string,bufferlist> *o,
+                             bufferlist *o,
                              std::vector<snapid_t> *osnaps,
                              snapid_t *osnap_seq,
                              uint32_t *flags,
@@ -642,7 +641,7 @@ struct ObjectOperation {
       : cursor(c),
        out_size(s), out_mtime(m),
        out_attrs(a), out_data(d), out_omap_header(oh),
-       out_omap(o), out_snaps(osnaps), out_snap_seq(osnap_seq),
+       out_omap_data(o), out_snaps(osnaps), out_snap_seq(osnap_seq),
        out_flags(flags), out_data_digest(dd), out_omap_digest(od),
        prval(r) {}
     void finish(int r) {
@@ -662,8 +661,8 @@ struct ObjectOperation {
          out_data->claim_append(copy_reply.data);
        if (out_omap_header)
          out_omap_header->claim_append(copy_reply.omap_header);
-       if (out_omap)
-         *out_omap = copy_reply.omap;
+       if (out_omap_data)
+         *out_omap_data = copy_reply.omap_data;
        if (out_snaps)
          *out_snaps = copy_reply.snaps;
        if (out_snap_seq)
@@ -689,7 +688,7 @@ struct ObjectOperation {
                std::map<std::string,bufferlist> *out_attrs,
                bufferlist *out_data,
                bufferlist *out_omap_header,
-               std::map<std::string,bufferlist> *out_omap,
+               bufferlist *out_omap_data,
                vector<snapid_t> *out_snaps,
                snapid_t *out_snap_seq,
                uint32_t *out_flags,
@@ -705,7 +704,7 @@ struct ObjectOperation {
     C_ObjectOperation_copyget *h =
       new C_ObjectOperation_copyget(cursor, out_size, out_mtime,
                                     out_attrs, out_data, out_omap_header,
-                                   out_omap, out_snaps, out_snap_seq,
+                                   out_omap_data, out_snaps, out_snap_seq,
                                    out_flags, out_data_digest, out_omap_digest,
                                    prval);
     out_bl[p] = &h->bl;

diff --git a/src/test/encoding/types.h b/src/test/encoding/types.h
index f85382157aeb13023b394b83066805302a1358f6..4e96f378fd3290d452d9870fbfe98b21a569ea92 100644 (file)
--- a/src/test/encoding/types.h
+++ b/src/test/encoding/types.h
@@ -65,7 +65,7 @@ TYPE(pg_missing_t)
 TYPE(pg_ls_response_t)
 TYPE(pg_nls_response_t)
 TYPE(object_copy_cursor_t)
-TYPE(object_copy_data_t)
+TYPE_FEATUREFUL(object_copy_data_t)
 TYPE(pg_create_t)
 TYPE(watch_info_t)
 TYPE(object_info_t)