]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
test: idempotent filestore test failure
authorDavid Zafman <david.zafman@inktank.com>
Fri, 12 Jul 2013 02:47:47 +0000 (19:47 -0700)
committerDavid Zafman <david.zafman@inktank.com>
Fri, 12 Jul 2013 21:21:21 +0000 (14:21 -0700)
Remove obsolete use of collection_move()
Allow operations to be skipped if random selections don't make sense
Track total number of possible objects in m_num_objects
BUG: do_remove() was calling _do_touch() instead of _do_remove()
For ops that require an object, select from among existing objects in collection
Initialize m_num_objects unique objects across collections

touch: don't create an object that already exists in another collection
remove: Use remove_obj() to clear object from m_objects to have accurate tracking
clone/clone_range(): Select 2 existing objects in the collection
add: Skip operation if selected target object name exists in target collection
move: Removed this buggy operation that is only present for upgrades

Fixes: #5371
Fixes: #5240
Signed-off-by: David Zafman <david.zafman@inktank.com>
src/test/filestore/DeterministicOpSequence.cc
src/test/filestore/DeterministicOpSequence.h
src/test/filestore/TestFileStoreState.cc
src/test/filestore/TestFileStoreState.h

index c347df3b2a367f073dbfd8453b85544594e42a6c..d9a0be43fc9046db3a5f8592937510525d45c80e 100644 (file)
@@ -52,39 +52,38 @@ DeterministicOpSequence::~DeterministicOpSequence()
   // TODO Auto-generated destructor stub
 }
 
-void DeterministicOpSequence::run_one_op(int op, rngen_t& gen)
+bool DeterministicOpSequence::run_one_op(int op, rngen_t& gen)
 {
+  bool ok = false;
   switch (op) {
   case DSOP_TOUCH:
-    do_touch(gen);
+    ok = do_touch(gen);
     break;
   case DSOP_WRITE:
-    do_write(gen);
+    ok = do_write(gen);
     break;
   case DSOP_CLONE:
-    do_clone(gen);
+    ok = do_clone(gen);
     break;
   case DSOP_CLONE_RANGE:
-    do_clone_range(gen);
+    ok = do_clone_range(gen);
     break;
   case DSOP_OBJ_REMOVE:
-    do_remove(gen);
-    break;
-  case DSOP_COLL_MOVE:
-    do_coll_move(gen);
+    ok = do_remove(gen);
     break;
   case DSOP_COLL_ADD:
-    do_coll_add(gen);
+    ok = do_coll_add(gen);
     break;
   case DSOP_COLL_RENAME:
     //do_coll_rename(gen);
     break;
   case DSOP_SET_ATTRS:
-    do_set_attrs(gen);
+    ok = do_set_attrs(gen);
     break;
   default:
     assert(0 == "bad op");
   }
+  return ok;
 }
 
 void DeterministicOpSequence::generate(int seed, int num_txs)
@@ -102,11 +101,12 @@ void DeterministicOpSequence::generate(int seed, int num_txs)
   rngen_t gen(seed);
   boost::uniform_int<> op_rng(DSOP_FIRST, DSOP_LAST);
 
-  for (txn = 1; txn <= num_txs; txn++) {
+  for (txn = 1; txn <= num_txs; ) {
     int op = op_rng(gen);
     _print_status(txn, op);
     dout(0) << "generate seq " << txn << " op " << op << dendl;
-    run_one_op(op, gen);
+    if (run_one_op(op, gen))
+      txn++;
   }
 }
 
@@ -126,7 +126,7 @@ int DeterministicOpSequence::_gen_coll_id(rngen_t& gen)
 
 int DeterministicOpSequence::_gen_obj_id(rngen_t& gen)
 {
-  boost::uniform_int<> obj_rng(0, m_num_objs_per_coll-1);
+  boost::uniform_int<> obj_rng(0, m_num_objects - 1);
   return obj_rng(gen);
 }
 
@@ -139,34 +139,59 @@ void DeterministicOpSequence::note_txn(ObjectStore::Transaction *t)
   dout(10) << __func__ << " " << txn << dendl;
 }
 
-void DeterministicOpSequence::do_touch(rngen_t& gen)
+bool DeterministicOpSequence::do_touch(rngen_t& gen)
 {
   int coll_id = _gen_coll_id(gen);
   int obj_id = _gen_obj_id(gen);
 
   coll_entry_t *entry = get_coll_at(coll_id);
   ceph_assert(entry != NULL);
+
+  // Don't care about other collections if already exists
+  if (!entry->check_for_obj(obj_id)) {
+    bool other_found = false;
+    map<int, coll_entry_t*>::iterator it = m_collections.begin();
+    for (; it != m_collections.end(); ++it) {
+      if (it->second->check_for_obj(obj_id)) {
+        ceph_assert(it->first != coll_id);
+        other_found = true;
+      }
+    }
+    if (other_found) {
+      dout(0) << "do_touch new object in collection and exists in another" << dendl;
+      return false;
+    }
+  }
   hobject_t *obj = entry->touch_obj(obj_id);
 
   dout(0) << "do_touch " << entry->m_coll.to_str() << "/" << obj->oid.name << dendl;
 
   _do_touch(entry->m_coll, *obj);
+  return true;
 }
 
-void DeterministicOpSequence::do_remove(rngen_t& gen)
+bool DeterministicOpSequence::do_remove(rngen_t& gen)
 {
   int coll_id = _gen_coll_id(gen);
-  int obj_id = _gen_obj_id(gen);
 
   coll_entry_t *entry = get_coll_at(coll_id);
   ceph_assert(entry != NULL);
-  hobject_t *obj = entry->touch_obj(obj_id);
 
-  // ENOENT ok here.
+  if (entry->m_objects.size() == 0) {
+    dout(0) << "do_remove no objects in collection" << dendl;
+    return false;
+  }
+  int obj_id = entry->get_random_obj_id(gen);
+  hobject_t *obj = entry->touch_obj(obj_id);
+  ceph_assert(obj);
 
   dout(0) << "do_remove " << entry->m_coll.to_str() << "/" << obj->oid.name << dendl;
 
-  _do_touch(entry->m_coll, *obj);
+  _do_remove(entry->m_coll, *obj);
+  hobject_t *rmobj = entry->remove_obj(obj_id);
+  ceph_assert(rmobj);
+  delete rmobj;
+  return true;
 }
 
 static void _gen_random(rngen_t& gen,
@@ -201,46 +226,43 @@ static void gen_attrs(rngen_t &gen,
   }
 }
 
-void DeterministicOpSequence::do_set_attrs(rngen_t& gen) {
+bool DeterministicOpSequence::do_set_attrs(rngen_t& gen) 
+{
   int coll_id = _gen_coll_id(gen);
-  int obj_id = _gen_obj_id(gen);
 
   coll_entry_t *entry = get_coll_at(coll_id);
-  if (!entry) {
-    dout(0) << "do_set_attrs coll id " << coll_id << " non-existent" << dendl;
-    return;
-  }
+  ceph_assert(entry != NULL);
 
-  hobject_t *obj = entry->get_obj(obj_id);
-  if (!obj) {
-    dout(0) << "do_set_attrs " << entry->m_coll.to_str()
-           << " no object #" << obj_id << dendl;
-    return;
+  if (entry->m_objects.size() == 0) {
+    dout(0) << "do_set_attrs no objects in collection" << dendl;
+    return false;
   }
+  int obj_id = entry->get_random_obj_id(gen);
+  hobject_t *obj = entry->touch_obj(obj_id);
+  ceph_assert(obj);
 
   map<string, bufferlist> out;
   gen_attrs(gen, &out);
 
   dout(0) << "do_set_attrs " << out.size() << " entries" << dendl;
-  return _do_set_attrs(entry->m_coll, *obj, out);
+  _do_set_attrs(entry->m_coll, *obj, out);
+  return true;
 }
 
-void DeterministicOpSequence::do_write(rngen_t& gen)
+bool DeterministicOpSequence::do_write(rngen_t& gen)
 {
   int coll_id = _gen_coll_id(gen);
-  int obj_id = _gen_obj_id(gen);
 
   coll_entry_t *entry = get_coll_at(coll_id);
-  if (!entry) {
-    dout(0) << "do_write coll id " << coll_id << " non-existent" << dendl;
-    return;
+  ceph_assert(entry != NULL);
+
+  if (entry->m_objects.size() == 0) {
+    dout(0) << "do_write no objects in collection" << dendl;
+    return false;
   }
+  int obj_id = entry->get_random_obj_id(gen);
   hobject_t *obj = entry->touch_obj(obj_id);
-  if (!obj) {
-    dout(0) << "do_write " << entry->m_coll.to_str()
-    << " no object #" << obj_id << dendl;
-    return;
-  }
+  ceph_assert(obj);
 
   boost::uniform_int<> size_rng(100, (2 << 19));
   size_t size = (size_t) size_rng(gen);
@@ -251,6 +273,7 @@ void DeterministicOpSequence::do_write(rngen_t& gen)
          << " 0~" << size << dendl;
 
   _do_write(entry->m_coll, *obj, 0, bl.length(), bl);
+  return true;
 }
 
 bool DeterministicOpSequence::_prepare_clone(rngen_t& gen,
@@ -259,27 +282,24 @@ bool DeterministicOpSequence::_prepare_clone(rngen_t& gen,
   int coll_id = _gen_coll_id(gen);
 
   coll_entry_t *entry = get_coll_at(coll_id);
-  if (!entry) {
-    dout(0) << "_prepare_clone coll id " << coll_id << " non-existent" << dendl;
-    return false;
-  }
+  ceph_assert(entry != NULL);
 
-  if (entry->m_objects.size() == 0) {
+  if (entry->m_objects.size() >= 2) {
     dout(0) << "_prepare_clone coll " << entry->m_coll.to_str()
-           << " has no objects to clone" << dendl;
+           << " doesn't have 2 or more objects" << dendl;
     return false;
   }
 
-  boost::uniform_int<> orig_obj_rng(0, entry->m_objects.size()-1);
-  int orig_obj_pos = orig_obj_rng(gen);
-  int orig_obj_id = -1;
-  hobject_t *orig_obj = entry->get_obj_at(orig_obj_pos, &orig_obj_id);
+  int orig_obj_id = entry->get_random_obj_id(gen);
+  hobject_t *orig_obj = entry->touch_obj(orig_obj_id);
+  ceph_assert(orig_obj);
 
   int id;
   do {
-    id = _gen_obj_id(gen);
+    id = entry->get_random_obj_id(gen);
   } while (id == orig_obj_id);
   hobject_t *new_obj = entry->touch_obj(id);
+  ceph_assert(new_obj);
 
   coll_ret = entry->m_coll;
   orig_obj_ret = *orig_obj;
@@ -288,26 +308,27 @@ bool DeterministicOpSequence::_prepare_clone(rngen_t& gen,
   return true;
 }
 
-void DeterministicOpSequence::do_clone(rngen_t& gen)
+bool DeterministicOpSequence::do_clone(rngen_t& gen)
 {
   coll_t coll;
   hobject_t orig_obj, new_obj;
   if (!_prepare_clone(gen, coll, orig_obj, new_obj)) {
-    return;
+    return false;
   }
 
   dout(0) << "do_clone " << coll.to_str() << "/" << orig_obj.oid.name
       << " => " << coll.to_str() << "/" << new_obj.oid.name << dendl;
 
   _do_clone(coll, orig_obj, new_obj);
+  return true;
 }
 
-void DeterministicOpSequence::do_clone_range(rngen_t& gen)
+bool DeterministicOpSequence::do_clone_range(rngen_t& gen)
 {
   coll_t coll;
   hobject_t orig_obj, new_obj;
   if (!_prepare_clone(gen, coll, orig_obj, new_obj)) {
-    return;
+    return false;
   }
 
   /* Whenever we have to make a clone_range() operation, just write to the
@@ -330,13 +351,15 @@ void DeterministicOpSequence::do_clone_range(rngen_t& gen)
       << " => " << coll.to_str() << "/" << new_obj.oid.name
       << " (0)" << dendl;
   _do_write_and_clone_range(coll, orig_obj, new_obj, 0, size, 0, bl);
+  return true;
 }
 
 bool DeterministicOpSequence::_prepare_colls(rngen_t& gen,
                                             coll_entry_t* &orig_coll, coll_entry_t* &new_coll)
 {
+  ceph_assert(m_collections_ids.size() > 1);
   int orig_coll_id = _gen_coll_id(gen);
-  int new_coll_id = orig_coll_id;
+  int new_coll_id;
   do {
     new_coll_id = _gen_coll_id(gen);
   } while (new_coll_id == orig_coll_id);
@@ -345,14 +368,9 @@ bool DeterministicOpSequence::_prepare_colls(rngen_t& gen,
       << " to coll id " << new_coll_id << dendl;
 
   orig_coll = get_coll_at(orig_coll_id);
+  ceph_assert(orig_coll != NULL);
   new_coll = get_coll_at(new_coll_id);
-
-  if (!orig_coll || !new_coll) {
-    dout(0) << "_prepare_colls " << " no such collection "
-        << "(orig: " << orig_coll << " id " << orig_coll_id
-        << " ; new: " << new_coll << " id" << new_coll_id << ")" << dendl;
-    return false;
-  }
+  ceph_assert(new_coll != NULL);
 
   if (!orig_coll->m_objects.size()) {
     dout(0) << "_prepare_colls coll " << orig_coll->m_coll.to_str()
@@ -364,34 +382,7 @@ bool DeterministicOpSequence::_prepare_colls(rngen_t& gen,
 }
 
 
-void DeterministicOpSequence::do_coll_move(rngen_t& gen)
-{
-  coll_entry_t *orig_coll = NULL, *new_coll = NULL;
-  if (!_prepare_colls(gen, orig_coll, new_coll))
-    return;
-
-  if (!orig_coll || !new_coll)
-    return;
-
-  boost::uniform_int<> obj_rng(0, orig_coll->m_objects.size()-1);
-  int obj_pos = obj_rng(gen);
-  int obj_key = -1;
-  hobject_t *obj = orig_coll->remove_obj_at(obj_pos, &obj_key);
-
-  hobject_t *new_coll_old_obj = new_coll->replace_obj(obj_key, obj);
-  dout(0) << "do_coll_move " << orig_coll->m_coll.to_str() << "/" << obj->oid.name
-         << " => " << new_coll->m_coll.to_str() << "/" << obj->oid.name << dendl;
-  if (new_coll_old_obj != NULL) {
-    dout(0) << "do_coll_move replacing obj " << new_coll->m_coll.to_str()
-           << "/" << new_coll_old_obj->oid.name
-           << " with " << obj->oid.name << dendl;
-    delete new_coll_old_obj;
-  }
-
-  _do_coll_move(new_coll->m_coll, orig_coll->m_coll, *obj);
-}
-
-void DeterministicOpSequence::do_coll_rename(rngen_t& gen)
+bool DeterministicOpSequence::do_coll_rename(rngen_t& gen)
 {
   int coll_pos = _gen_coll_id(gen);
   dout(0) << "do_coll_rename coll pos #" << coll_pos << dendl;
@@ -399,7 +390,7 @@ void DeterministicOpSequence::do_coll_rename(rngen_t& gen)
   coll_entry_t *coll_entry = get_coll_at(coll_pos);
   if (!coll_entry) {
     dout(0) << "do_coll_rename no collection at pos #" << coll_pos << dendl;
-    return;
+    return false;
   }
 
   coll_t orig_coll = coll_entry->m_coll;
@@ -412,16 +403,16 @@ void DeterministicOpSequence::do_coll_rename(rngen_t& gen)
   dout(0) << "do_coll_rename " << orig_coll.to_str()
       << " => " << new_coll.to_str() << dendl;
   _do_coll_rename(orig_coll, new_coll);
+  return true;
 }
 
-void DeterministicOpSequence::do_coll_add(rngen_t& gen)
+bool DeterministicOpSequence::do_coll_add(rngen_t& gen)
 {
   coll_entry_t *orig_coll = NULL, *new_coll = NULL;
   if (!_prepare_colls(gen, orig_coll, new_coll))
-    return;
+    return false;
 
-  if (!orig_coll || !new_coll)
-    return;
+  ceph_assert(orig_coll && new_coll);
 
   boost::uniform_int<> obj_rng(0, orig_coll->m_objects.size()-1);
   int obj_pos = obj_rng(gen);
@@ -431,13 +422,20 @@ void DeterministicOpSequence::do_coll_add(rngen_t& gen)
     dout(0) << "do_coll_add coll " << orig_coll->m_coll.to_str()
         << " has no object as pos #" << obj_pos << " (key " << obj_key << ")"
         << dendl;
-    return;
+    return false;
+  }
+  if (new_coll->check_for_obj(obj_key)) {
+    dout(0) << "do_coll_add coll " << orig_coll->m_coll.to_str()
+        << " already has object as pos #" << obj_pos << " (key " << obj_key << ")"
+        << dendl;
+    return false;
   }
   dout(0) << "do_coll_add " << orig_coll->m_coll.to_str() << "/" << obj->oid.name
         << " => " << new_coll->m_coll.to_str() << "/" << obj->oid.name << dendl;
   new_coll->touch_obj(obj_key);
 
   _do_coll_add(orig_coll->m_coll, new_coll->m_coll, *obj);
+  return true;
 }
 
 void DeterministicOpSequence::_do_touch(coll_t coll, hobject_t& obj)
@@ -509,16 +507,6 @@ void DeterministicOpSequence::_do_write_and_clone_range(coll_t coll,
   m_store->apply_transaction(t);
 }
 
-void DeterministicOpSequence::_do_coll_move(coll_t new_coll,
-                                           coll_t old_coll, hobject_t& obj)
-{
-  ObjectStore::Transaction t;
-  note_txn(&t);
-  t.remove(new_coll, obj);
-  t.collection_move(new_coll, old_coll, obj);
-  m_store->apply_transaction(t);
-}
-
 void DeterministicOpSequence::_do_coll_add(coll_t orig_coll, coll_t new_coll,
                                           hobject_t& obj)
 {
index 44e632ab5c081fbda3f4aad09493a7181ce3465a..818d0ed6448955612d9acb04789f47ad7a03dbd0 100644 (file)
@@ -39,10 +39,9 @@ class DeterministicOpSequence : public TestFileStoreState {
     DSOP_CLONE = 2,
     DSOP_CLONE_RANGE = 3,
     DSOP_OBJ_REMOVE = 4,
-    DSOP_COLL_MOVE = 5,
-    DSOP_COLL_RENAME = 6,
-    DSOP_COLL_ADD = 7,
-    DSOP_SET_ATTRS = 8,
+    DSOP_COLL_RENAME = 5,
+    DSOP_COLL_ADD = 6,
+    DSOP_SET_ATTRS = 7,
 
     DSOP_FIRST = DSOP_TOUCH,
     DSOP_LAST = DSOP_SET_ATTRS,
@@ -56,18 +55,17 @@ class DeterministicOpSequence : public TestFileStoreState {
   ObjectStore::Sequencer m_osr;
   std::ofstream m_status;
 
-  void run_one_op(int op, rngen_t& gen);
+  bool run_one_op(int op, rngen_t& gen);
 
   void note_txn(ObjectStore::Transaction *t);
-  void do_touch(rngen_t& gen);
-  void do_remove(rngen_t& gen);
-  void do_write(rngen_t& gen);
-  void do_clone(rngen_t& gen);
-  void do_clone_range(rngen_t& gen);
-  void do_coll_move(rngen_t& gen);
-  void do_coll_rename(rngen_t& gen);
-  void do_coll_add(rngen_t& gen);
-  void do_set_attrs(rngen_t& gen);
+  bool do_touch(rngen_t& gen);
+  bool do_remove(rngen_t& gen);
+  bool do_write(rngen_t& gen);
+  bool do_clone(rngen_t& gen);
+  bool do_clone_range(rngen_t& gen);
+  bool do_coll_rename(rngen_t& gen);
+  bool do_coll_add(rngen_t& gen);
+  bool do_set_attrs(rngen_t& gen);
 
   virtual void _do_touch(coll_t coll, hobject_t& obj);
   virtual void _do_remove(coll_t coll, hobject_t& obj);
@@ -82,7 +80,6 @@ class DeterministicOpSequence : public TestFileStoreState {
   virtual void _do_write_and_clone_range(coll_t coll, hobject_t& orig_obj,
       hobject_t& new_obj, uint64_t srcoff, uint64_t srclen,
       uint64_t dstoff, bufferlist& bl);
-  virtual void _do_coll_move(coll_t new_coll, coll_t old_coll, hobject_t& obj);
   virtual void _do_coll_add(coll_t orig_coll, coll_t new_coll, hobject_t& obj);
   virtual void _do_coll_rename(coll_t orig_coll, coll_t new_coll);
 
index c786f87f268268c216e8b82cf8e13c5a51d8ac70..a34e52636cebb46fff0f2ead542be630327a8692 100644 (file)
@@ -45,6 +45,7 @@ void TestFileStoreState::init(int colls, int objs)
 
   wait_for_ready();
 
+  int baseid = 0;
   for (int i = 0; i < colls; i++) {
     int coll_id = i;
     coll_entry_t *entry = coll_create(coll_id);
@@ -56,9 +57,12 @@ void TestFileStoreState::init(int colls, int objs)
     t->touch(META_COLL, entry->m_meta_obj);
 
     for (int i = 0; i < objs; i++) {
-      hobject_t *obj = entry->touch_obj(i);
+      hobject_t *obj = entry->touch_obj(i + baseid);
       t->touch(entry->m_coll, *obj);
+      ceph_assert(i + baseid == m_num_objects);
+      m_num_objects++;
     }
+    baseid += objs;
 
     m_store->queue_transaction(&(entry->m_osr), t,
         new C_OnFinished(this, t));
@@ -157,6 +161,13 @@ TestFileStoreState::coll_entry_t::~coll_entry_t()
   }
 }
 
+bool TestFileStoreState::coll_entry_t::check_for_obj(int id)
+{
+  if (m_objects.count(id))
+    return true;
+  return false;
+}
+
 hobject_t *TestFileStoreState::coll_entry_t::touch_obj(int id)
 {
   map<int, hobject_t*>::iterator it = m_objects.find(id);
@@ -268,3 +279,18 @@ TestFileStoreState::coll_entry_t::replace_obj(int id, hobject_t *obj) {
   m_objects.insert(make_pair(id, obj));
   return old_obj;
 }
+
+int TestFileStoreState::coll_entry_t::get_random_obj_id(rngen_t& gen)
+{
+  ceph_assert(!m_objects.empty());
+
+  boost::uniform_int<> orig_obj_rng(0, m_objects.size()-1);
+  int pos = orig_obj_rng(gen);
+  map<int, hobject_t*>::iterator it = m_objects.begin();
+  for (int i = 0; it != m_objects.end(); ++it, i++) {
+    if (i == pos) {
+      return it->first;
+    }
+  }
+  ceph_assert(0 == "INTERNAL ERROR");
+}
index d3bba692437100cc7c36a459eb6e139ce76d028f..7cd4527bccc5101f52da50eab6c377c9e89727d1 100644 (file)
@@ -20,6 +20,8 @@
 #include <map>
 #include <vector>
 
+typedef boost::mt11213b rngen_t;
+
 class TestFileStoreState {
 public:
   struct coll_entry_t {
@@ -38,11 +40,13 @@ public:
     ~coll_entry_t();
 
     hobject_t *touch_obj(int id);
+    bool check_for_obj(int id);
     hobject_t *get_obj(int id);
     hobject_t *remove_obj(int id);
     hobject_t *get_obj_at(int pos, int *key = NULL);
     hobject_t *remove_obj_at(int pos, int *key = NULL);
     hobject_t *replace_obj(int id, hobject_t *obj);
+    int get_random_obj_id(rngen_t& gen);
 
    private:
     hobject_t *get_obj(int id, bool remove);
@@ -59,6 +63,7 @@ public:
   vector<int> m_collections_ids;
   int m_next_coll_nr;
   int m_num_objs_per_coll;
+  int m_num_objects;
 
   int m_max_in_flight;
   atomic_t m_in_flight;
@@ -92,7 +97,7 @@ public:
 
  public:
   TestFileStoreState(FileStore *store) :
-    m_next_coll_nr(0), m_num_objs_per_coll(10),
+    m_next_coll_nr(0), m_num_objs_per_coll(10), m_num_objects(0),
     m_max_in_flight(0), m_finished_lock("Finished Lock") {
     m_in_flight.set(0);
     m_store.reset(store);