filestore: remove btrfs start/end transaction ioctl code

This was an ill-conceived approach to getting atomic transactions out of
btrfs.  It doesn't offer rollback, which means that any error means we need
to wedge the file system and reboot in order to avoid corrupting the
data set.  And that's silly!

Snapshots are more robust and only marginally slower (because we have to
quiesce our writes while waiting for the snap to start, and btrfs resume
work in-kernel slightly faster...maybe).

Fixes: #2623
Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index bd6c39aedc62f7fce538db104e3d166008152874..b671807d27ba402813e367885013778ca96e45e6 100644 (file)
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -347,7 +347,6 @@ OPTION(filestore_max_inline_xattrs, OPT_U32, 2)
 
 OPTION(filestore_max_sync_interval, OPT_DOUBLE, 5)    // seconds
 OPTION(filestore_min_sync_interval, OPT_DOUBLE, .01)  // seconds
-OPTION(filestore_btrfs_trans, OPT_BOOL, false)
 OPTION(filestore_btrfs_snap, OPT_BOOL, true)
 OPTION(filestore_btrfs_clone_range, OPT_BOOL, true)
 OPTION(filestore_fsync_flushes_journal_data, OPT_BOOL, false)

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 41433e7683977334acd207a18c9ddc702b3f409b..2f88157461cfa33e6e743beb1055ddf944e6ad10 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -690,7 +690,6 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, const cha
   logger(NULL),
   m_filestore_btrfs_clone_range(g_conf->filestore_btrfs_clone_range),
   m_filestore_btrfs_snap (g_conf->filestore_btrfs_snap ),
-  m_filestore_btrfs_trans(g_conf->filestore_btrfs_trans),
   m_filestore_commit_timeout(g_conf->filestore_commit_timeout),
   m_filestore_fiemap(g_conf->filestore_fiemap),
   m_filestore_flusher (g_conf->filestore_flusher ),
@@ -2366,11 +2365,6 @@ int FileStore::do_transactions(list<Transaction*> &tls, uint64_t op_seq)
     ops += (*p)->get_num_ops();
   }
 
-  int id = _transaction_start(bytes, ops);
-  if (id < 0) {
-    return id;
-  }
-    
   int trans_num = 0;
   for (list<Transaction*>::iterator p = tls.begin();
        p != tls.end();
@@ -2380,7 +2374,6 @@ int FileStore::do_transactions(list<Transaction*> &tls, uint64_t op_seq)
       break;
   }
   
-  _transaction_finish(id);
   return r;
 }
 
@@ -2414,52 +2407,6 @@ unsigned FileStore::apply_transactions(list<Transaction*> &tls,
 }
 
 
-// btrfs transaction start/end interface
-
-int FileStore::_transaction_start(uint64_t bytes, uint64_t ops)
-{
-  if (!btrfs || !btrfs_trans_start_end ||
-      !m_filestore_btrfs_trans)
-    return 0;
-
-  int fd = ::open(basedir.c_str(), O_RDONLY);
-  if (fd < 0) {
-    int err = errno;
-    dout(0) << "transaction_start got " << cpp_strerror(err) << " from btrfs open" << dendl;
-    assert(0 == "couldn't open basedir");
-  }
-
-  int r = ::ioctl(fd, BTRFS_IOC_TRANS_START);
-  if (r < 0) {
-    int err = errno;
-    dout(0) << "transaction_start got " << cpp_strerror(err) << " from btrfs ioctl" << dendl;    
-    TEMP_FAILURE_RETRY(::close(fd));
-    return -err;
-  }
-  dout(10) << "transaction_start " << fd << dendl;
-
-  char fn[PATH_MAX];
-  snprintf(fn, sizeof(fn), "%s/current/trans.%d", basedir.c_str(), fd);
-  ::mknod(fn, 0644, 0);
-
-  return fd;
-}
-
-void FileStore::_transaction_finish(int fd)
-{
-  if (!btrfs || !btrfs_trans_start_end ||
-      !m_filestore_btrfs_trans)
-    return;
-
-  char fn[PATH_MAX];
-  snprintf(fn, sizeof(fn), "%s/current/trans.%d", basedir.c_str(), fd);
-  ::unlink(fn);
-  
-  dout(10) << "transaction_finish " << fd << dendl;
-  ::ioctl(fd, BTRFS_IOC_TRANS_END);
-  TEMP_FAILURE_RETRY(::close(fd));
-}
-
 void FileStore::_set_replay_guard(int fd,
                                  const SequencerPosition& spos,
                                  const hobject_t *hoid,

diff --git a/src/os/FileStore.h b/src/os/FileStore.h
index 236430dde3e8027d47f5d2be13f51416f1d65741..e98301c67bba8e6669a1b44c7988ed013fb436df 100644 (file)
--- a/src/os/FileStore.h
+++ b/src/os/FileStore.h
@@ -314,8 +314,6 @@ public:
   int do_transactions(list<Transaction*> &tls, uint64_t op_seq);
   unsigned apply_transaction(Transaction& t, Context *ondisk=0);
   unsigned apply_transactions(list<Transaction*>& tls, Context *ondisk=0);
-  int _transaction_start(uint64_t bytes, uint64_t ops);
-  void _transaction_finish(int id);
   unsigned _do_transaction(Transaction& t, uint64_t op_seq, int trans_num);
 
   int queue_transaction(Sequencer *osr, Transaction* t);
@@ -477,7 +475,6 @@ private:
                          const std::set <std::string> &changed);
   bool m_filestore_btrfs_clone_range;
   bool m_filestore_btrfs_snap;
-  bool m_filestore_btrfs_trans;
   float m_filestore_commit_timeout;
   bool m_filestore_fiemap;
   bool m_filestore_flusher;