osd/scrub: modify SnapMapper.cc to use ceph::buffer::list

... systematically, over ceph::bufferlist.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/qa/standalone/scrub/osd-mapper.sh b/qa/standalone/scrub/osd-mapper.sh
index 989271b9158cb035ac78819ba17509f6f946e48f..ed18f94f1af1f37298a6d6bd6ae107da9c5bd46d 100755 (executable)
--- a/qa/standalone/scrub/osd-mapper.sh
+++ b/qa/standalone/scrub/osd-mapper.sh
@@ -49,7 +49,7 @@ function TEST_truncated_sna_record() {
         ['pool_name']="test"
     )
 
-    local extr_dbg=1
+    local extr_dbg=3
     (( extr_dbg > 1 )) && echo "Dir: $dir"
     standard_scrub_cluster $dir cluster_conf
     ceph tell osd.* config set osd_stats_update_period_not_scrubbing "1"
@@ -100,6 +100,8 @@ function TEST_truncated_sna_record() {
     (( extr_dbg >= 2 )) && ceph-kvstore-tool bluestore-kv $dir/2 dump "p" | grep -a SNA_
     (( extr_dbg >= 2 )) && grep -a SNA_ /tmp/oo2.dump
     (( extr_dbg >= 2 )) && ceph-kvstore-tool bluestore-kv $dir/2 dump p 2> /dev/null
+    local num_sna_b4=`ceph-kvstore-tool bluestore-kv $dir/$osd dump p 2> /dev/null | grep -a -e 'SNA_[0-9]_000000000000000[0-9]_000000000000000' \
+            | awk -e '{print $2;}' | wc -l`
 
     for sdn in $(seq 0 $(expr $osdn - 1))
     do
@@ -137,6 +139,7 @@ function TEST_truncated_sna_record() {
     do
       timeout 60 ceph tell osd.$sdn version
     done
+    rados --format json-pretty -p $poolname listsnaps $objname
 
     # when scrubbing now - we expect the scrub to emit a cluster log ERR message regarding SnapMapper internal inconsistency
     ceph osd unset nodeep-scrub || return 1
@@ -151,8 +154,29 @@ function TEST_truncated_sna_record() {
     ceph pg dump pgs
     (( extr_dbg >= 1 )) && grep -a "ERR" $dir/osd.$cur_prim.log
     grep -a -q "ERR" $dir/osd.$cur_prim.log || return 1
-}
 
+    # but did we fix the snap issue? let's try scrubbing again
+
+    local prev_err_cnt=`grep -a "ERR" $dir/osd.$cur_prim.log | wc -l`
+    echo "prev count: $prev_err_cnt"
+
+    # scrub again. No errors expected this time
+    ceph pg $pgid deep_scrub || return 1
+    sleep 5
+    ceph pg dump pgs
+    (( extr_dbg >= 1 )) && grep -a "ERR" $dir/osd.$cur_prim.log
+    local current_err_cnt=`grep -a "ERR" $dir/osd.$cur_prim.log | wc -l`
+    (( extr_dbg >= 1 )) && echo "current count: $current_err_cnt"
+    (( current_err_cnt == prev_err_cnt )) || return 1
+    kill_daemons $dir TERM osd || return 1
+    kvdir=$dir/$cur_prim
+    (( extr_dbg >= 2 )) && ceph-kvstore-tool bluestore-kv $kvdir dump p 2> /dev/null | grep -a -e 'SNA_[0-9]_' \
+            | awk -e '{print $2;}'
+    local num_sna_full=`ceph-kvstore-tool bluestore-kv $kvdir dump p 2> /dev/null | grep -a -e 'SNA_[0-9]_000000000000000[0-9]_000000000000000' \
+            | awk -e '{print $2;}' | wc -l`
+    (( num_sna_full == num_sna_b4 )) || return 1
+    return 0
+}
 
 
 main osd-mapper "$@"

diff --git a/src/osd/SnapMapper.cc b/src/osd/SnapMapper.cc
index b943c1e5d71d1e0dc616bedc5309fe5e25a3d701..076f6fc8e71cadd812079754523663b9e4055621 100644 (file)
--- a/src/osd/SnapMapper.cc
+++ b/src/osd/SnapMapper.cc
@@ -32,7 +32,6 @@ using std::set;
 using std::string;
 using std::vector;
 
-using ceph::bufferlist;
 using ceph::decode;
 using ceph::encode;
 using ceph::timespan_str;
@@ -81,14 +80,14 @@ const char *SnapMapper::PURGED_SNAP_PREFIX = "PSN_";
 
 int OSDriver::get_keys(
   const std::set<std::string> &keys,
-  std::map<std::string, bufferlist> *out)
+  std::map<std::string, ceph::buffer::list> *out)
 {
   return os->omap_get_values(ch, hoid, keys, out);
 }
 
 int OSDriver::get_next(
   const std::string &key,
-  pair<std::string, bufferlist> *next)
+  pair<std::string, ceph::buffer::list> *next)
 {
   ObjectMap::ObjectMapIterator iter =
     os->get_omap_iterator(ch, hoid);
@@ -126,20 +125,20 @@ std::string SnapMapper::to_raw_key(snapid_t snap, const hobject_t &clone) const
   return get_prefix(clone.pool, snap) + shard_prefix + clone.to_str();
 }
 
-pair<string, bufferlist> SnapMapper::to_raw(
+pair<string, ceph::buffer::list> SnapMapper::to_raw(
   const pair<snapid_t, hobject_t> &in) const
 {
-  bufferlist bl;
+  ceph::buffer::list bl;
   encode(Mapping(in), bl);
   return make_pair(to_raw_key(in), bl);
 }
 
 pair<snapid_t, hobject_t> SnapMapper::from_raw(
-  const pair<std::string, bufferlist> &image)
+  const pair<std::string, ceph::buffer::list> &image)
 {
   using ceph::decode;
   Mapping map;
-  bufferlist bl(image.second);
+  ceph::buffer::list bl(image.second);
   auto bp = bl.cbegin();
   decode(map, bp);
   return make_pair(map.snap, map.hoid);
@@ -165,7 +164,7 @@ string SnapMapper::to_object_key(const hobject_t &hoid) const
   return OBJECT_PREFIX + shard_prefix + hoid.to_str();
 }
 
-void SnapMapper::object_snaps::encode(bufferlist &bl) const
+void SnapMapper::object_snaps::encode(ceph::buffer::list &bl) const
 {
   ENCODE_START(1, 1, bl);
   encode(oid, bl);
@@ -173,7 +172,7 @@ void SnapMapper::object_snaps::encode(bufferlist &bl) const
   ENCODE_FINISH(bl);
 }
 
-void SnapMapper::object_snaps::decode(bufferlist::const_iterator &bl)
+void SnapMapper::object_snaps::decode(ceph::buffer::list::const_iterator &bl)
 {
   DECODE_START(1, bl);
   decode(oid, bl);
@@ -232,7 +231,7 @@ SnapMapper::get_snaps_common(const hobject_t &oid) const
   dout(20) << fmt::format("{}: key string: {} oid:{}", __func__, keys, oid)
           << dendl;
 
-  map<string, bufferlist> got;
+  map<string, ceph::buffer::list> got;
   int r = backend.get_keys(keys, &got);
   if (r < 0) {
     dout(10) << __func__ << " " << oid << " got err " << r << dendl;
@@ -288,8 +287,8 @@ SnapMapper::get_snaps_check_consistency(const hobject_t &hoid) const
   // we have the clone oid and the set of snaps relevant to this clone.
   // Let's construct all expected SNA_ key, then fetch them.
 
-  map<string, bufferlist> kvmap;
   auto mapping_keys = to_raw_keys(hoid, *obj_snaps);
+  map<string, ceph::buffer::list> kvmap;
   auto r = backend.get_keys(mapping_keys, &kvmap);
   if (r < 0) {
     dout(10) << fmt::format(
@@ -331,7 +330,7 @@ SnapMapper::get_snaps_check_consistency(const hobject_t &hoid) const
 
 void SnapMapper::clear_snaps(
   const hobject_t &oid,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   dout(20) << __func__ << " " << oid << dendl;
   ceph_assert(check(oid));
@@ -348,11 +347,11 @@ void SnapMapper::clear_snaps(
 void SnapMapper::set_snaps(
   const hobject_t &oid,
   const object_snaps &in,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   ceph_assert(check(oid));
-  map<string, bufferlist> to_set;
-  bufferlist bl;
+  map<string, ceph::buffer::list> to_set;
+  ceph::buffer::list bl;
   encode(in, bl);
   to_set[to_object_key(oid)] = bl;
   dout(20) << __func__ << " " << oid << " " << in.snaps << dendl;
@@ -368,7 +367,7 @@ int SnapMapper::update_snaps(
   const hobject_t &oid,
   const set<snapid_t> &new_snaps,
   const set<snapid_t> *old_snaps_check,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   dout(20) << __func__ << " " << oid << " " << new_snaps
           << " was " << (old_snaps_check ? *old_snaps_check : set<snapid_t>())
@@ -408,7 +407,7 @@ int SnapMapper::update_snaps(
 void SnapMapper::add_oid(
   const hobject_t &oid,
   const set<snapid_t>& snaps,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   dout(20) << __func__ << " " << oid << " " << snaps << dendl;
   ceph_assert(!snaps.empty());
@@ -427,7 +426,7 @@ void SnapMapper::add_oid(
   object_snaps _snaps(oid, snaps);
   set_snaps(oid, _snaps, t);
 
-  map<string, bufferlist> to_add;
+  map<string, ceph::buffer::list> to_add;
   for (set<snapid_t>::iterator i = snaps.begin();
        i != snaps.end();
        ++i) {
@@ -461,7 +460,7 @@ int SnapMapper::get_next_objects_to_trim(
     string prefix(get_prefix(pool, snap) + *i);
     string pos = prefix;
     while (out->size() < max) {
-      pair<string, bufferlist> next;
+      pair<string, ceph::buffer::list> next;
       r = backend.get_next(pos, &next);
       dout(20) << __func__ << " get_next(" << pos << ") returns " << r
               << " " << next << dendl;
@@ -495,7 +494,7 @@ int SnapMapper::get_next_objects_to_trim(
 
 int SnapMapper::remove_oid(
   const hobject_t &oid,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   dout(20) << __func__ << " " << oid << dendl;
   ceph_assert(check(oid));
@@ -504,7 +503,7 @@ int SnapMapper::remove_oid(
 
 int SnapMapper::_remove_oid(
   const hobject_t &oid,
-  MapCacher::Transaction<std::string, bufferlist> *t)
+  MapCacher::Transaction<std::string, ceph::buffer::list> *t)
 {
   dout(20) << __func__ << " " << oid << dendl;
   object_snaps out;
@@ -555,7 +554,7 @@ string SnapMapper::make_purged_snap_key(int64_t pool, snapid_t last)
 }
 
 void SnapMapper::make_purged_snap_key_value(
-  int64_t pool, snapid_t begin, snapid_t end, map<string,bufferlist> *m)
+  int64_t pool, snapid_t begin, snapid_t end, map<string,ceph::buffer::list> *m)
 {
   string k = make_purged_snap_key(pool, end - 1);
   auto& v = (*m)[k];
@@ -586,7 +585,7 @@ int SnapMapper::_lookup_purged_snap(
             << it->key() << "'" << dendl;
     return -ENOENT;
   }
-  bufferlist v = it->value();
+  ceph::buffer::list v = it->value();
   auto p = v.cbegin();
   int64_t gotpool;
   decode(gotpool, p);
@@ -609,7 +608,7 @@ void SnapMapper::record_purged_snaps(
   map<epoch_t,mempool::osdmap::map<int64_t,snap_interval_set_t>> purged_snaps)
 {
   dout(10) << __func__ << " purged_snaps " << purged_snaps << dendl;
-  map<string,bufferlist> m;
+  map<string,ceph::buffer::list> m;
   set<string> rm;
   for (auto& [epoch, bypool] : purged_snaps) {
     // index by (pool, snap)
@@ -668,7 +667,7 @@ bool SnapMapper::Scrubber::_parse_p()
     pool = -1;
     return false;
   }
-  bufferlist v = psit->value();
+  ceph::buffer::list v = psit->value();
   auto p = v.cbegin();
   ceph::decode(pool, p);
   ceph::decode(begin, p);
@@ -808,7 +807,7 @@ bool SnapMapper::is_legacy_mapping(const string &to_test)
  */
 std::string SnapMapper::convert_legacy_key(
   const std::string& old_key,
-  const bufferlist& value)
+  const ceph::buffer::list& value)
 {
   auto old = from_raw(make_pair(old_key, value));
   std::string object_suffix = old_key.substr(
@@ -834,7 +833,7 @@ int SnapMapper::convert_legacy(
   auto start = ceph::mono_clock::now();
 
   iter->upper_bound(SnapMapper::LEGACY_MAPPING_PREFIX);
-  map<string,bufferlist> to_set;
+  map<string,ceph::buffer::list> to_set;
   while (iter->valid()) {
     bool valid = SnapMapper::is_legacy_mapping(iter->key());
     if (valid) {

diff --git a/src/osd/SnapMapper.h b/src/osd/SnapMapper.h
index 45d03298b2fb5415f10f3ee62be9b46aff00b487..01879c6e15b841d30528b82c42d0875cc70fe42c 100644 (file)
--- a/src/osd/SnapMapper.h
+++ b/src/osd/SnapMapper.h
@@ -60,11 +60,6 @@ public:
     }
   };
 
-  OSTransaction get_transaction(
-    ObjectStore::Transaction *t) {
-    return OSTransaction(ch->cid, hoid, t);
-  }
-
   OSTransaction get_transaction(
     ObjectStore::Transaction *t) const {
     return OSTransaction(ch->cid, hoid, t);