Merge pull request #7242 from xiexingguo/xxg-wip-14370

os/memstore: drain finisher first before we really begin a umount-process
os/bluestore: drain wal_wq on replay error

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

src/os/bluestore/BlueStore.cc

@@ -97,11 +97,11 @@ const string PREFIX_ALLOC = "B";   // u64 offset -> u64 length (freelist)
  * string encoding in the key
  *
  * The key string needs to lexicographically sort the same way that
- * ghobject_t does.  We do this by escaping anything <= to '%' with %
+ * ghobject_t does.  We do this by escaping anything <= to '#' with #
  * plus a 2 digit hex string, and anything >= '~' with ~ plus the two
  * hex digits.
  *
- * We use ! as a terminator for strings; this works because it is < %
+ * We use ! as a terminator for strings; this works because it is < #
  * and will get escaped if it is present in the string.
  *
  */
@@ -854,7 +854,7 @@ int BlueStore::_write_bdev_label(string path, bluestore_bdev_label_t label)
 
   int fd = ::open(path.c_str(), O_WRONLY);
   if (fd < 0) {
-    fd = errno;
+    fd = -errno;
     derr << __func__ << " failed to open " << path << ": " << cpp_strerror(fd)
 	 << dendl;
     return fd;
@@ -1613,7 +1613,7 @@ int BlueStore::mkfs()
     goto out_close_fsid;
 
   r = _read_fsid(&old_fsid);
-  if (r < 0 && old_fsid.is_zero()) {
+  if (r < 0 || old_fsid.is_zero()) {
     if (fsid.is_zero()) {
       fsid.generate_random();
       dout(1) << __func__ << " generated fsid " << fsid << dendl;
@@ -1799,10 +1799,11 @@ int BlueStore::mount()
 
  out_stop:
   _kv_stop();
+  wal_wq.drain();
   wal_tp.stop();
   finisher.wait_for_empty();
   finisher.stop();
-out_coll:
+ out_coll:
   coll_map.clear();
  out_alloc:
   _close_alloc();
@@ -2189,7 +2190,8 @@ int BlueStore::fsck()
       } catch (buffer::error& e) {
 	derr << __func__ << " failed to decode wal txn "
 	     << pretty_binary_string(it->key()) << dendl;
-	return -EIO;
+	r = -EIO;
+        goto out_scan;
       }
       dout(20) << __func__ << "  wal " << wt.seq
 	       << " ops " << wt.ops.size()
@@ -2224,6 +2226,7 @@ int BlueStore::fsck()
     }
   }
 
+ out_scan:
   coll_map.clear();
  out_alloc:
   _close_alloc();
@@ -2323,7 +2326,6 @@ void BlueStore::_reap_collections()
   }
 
   dout(10) << __func__ << " all reaped" << dendl;
-  reap_cond.Signal();
 }
 
 // ---------------
@@ -3862,17 +3864,19 @@ int BlueStore::_wal_replay()
   for (it->lower_bound(string()); it->valid(); it->next(), ++count) {
     dout(20) << __func__ << " replay " << pretty_binary_string(it->key())
 	     << dendl;
-    TransContext *txc = _txc_create(osr.get());
-    txc->wal_txn = new bluestore_wal_transaction_t;
+    bluestore_wal_transaction_t *wal_txn = new bluestore_wal_transaction_t;
     bufferlist bl = it->value();
     bufferlist::iterator p = bl.begin();
     try {
-      ::decode(*txc->wal_txn, p);
+      ::decode(*wal_txn, p);
     } catch (buffer::error& e) {
       derr << __func__ << " failed to decode wal txn "
 	   << pretty_binary_string(it->key()) << dendl;
+      delete wal_txn;
       return -EIO;
     }
+    TransContext *txc = _txc_create(osr.get());
+    txc->wal_txn = wal_txn;
     txc->state = TransContext::STATE_KV_DONE;
     _txc_state_proc(txc);
   }

src/os/bluestore/BlueStore.h

@@ -494,7 +494,6 @@ private:
   Logger *logger;
 
   Mutex reap_lock;
-  Cond reap_cond;
   list<CollectionRef> removed_collections;
 
 

src/os/kstore/KStore.cc

@@ -952,6 +952,10 @@ int KStore::mkfs()
   if (r < 0)
     goto out_close_db;
 
+  r = write_meta("type", "kstore");
+  if (r < 0)
+    goto out_close_db;
+
   // indicate mkfs completion/success by writing the fsid file
   r = _write_fsid();
   if (r == 0)
@@ -1429,7 +1433,6 @@ void KStore::_reap_collections()
   }
 
   dout(10) << __func__ << " all reaped" << dendl;
-  reap_cond.Signal();
 }
 
 // ---------------

src/os/kstore/KStore.h

@@ -309,7 +309,6 @@ private:
   Logger *logger;
 
   Mutex reap_lock;
-  Cond reap_cond;
   list<CollectionRef> removed_collections;
 
 

src/os/memstore/MemStore.cc

@@ -52,6 +52,7 @@ int MemStore::mount()
 
 int MemStore::umount()
 {
+  finisher.wait_for_empty();
   finisher.stop();
   return _save();
 }
@@ -59,7 +60,6 @@ int MemStore::umount()
 int MemStore::_save()
 {
   dout(10) << __func__ << dendl;
-  Mutex::Locker l(apply_lock); // block any writer
   dump_all();
   set<coll_t> collections;
   for (ceph::unordered_map<coll_t,CollectionRef>::iterator p = coll_map.begin();

src/os/memstore/MemStore.h

@@ -292,7 +292,6 @@ private:
 
   ceph::unordered_map<coll_t, CollectionRef> coll_map;
   RWLock coll_lock;    ///< rwlock to protect coll_map
-  Mutex apply_lock;    ///< serialize all updates
 
   CollectionRef get_collection(coll_t cid);
 
@@ -342,7 +341,6 @@ public:
     : ObjectStore(path),
       cct(cct),
       coll_lock("MemStore::coll_lock"),
-      apply_lock("MemStore::apply_lock"),
       finisher(cct),
       used_bytes(0) {}
   ~MemStore() { }