Atomize RenameFile in KeyManagedEncryptedEnv (#222)

* adjust logic in KeyManagedEncryptedEnv::RenameFile to avoid poweroff Signed-off-by: Xintao <[email protected]>
tikv · Dec 18, 2020 · 93e89a5 · 93e89a5
1 parent 7d209a8
commit 93e89a5
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 26 deletions.
diff --git a/db/db_test_util.h b/db/db_test_util.h
@@ -85,9 +85,6 @@ class TestKeyManager : public encryption::KeyManager {
   Status LinkFile(const std::string&, const std::string&) override {
     return Status::OK();
   }
-  Status RenameFile(const std::string&, const std::string&) override {
-    return Status::OK();
-  }
 };
 #endif
 

diff --git a/encryption/encryption.cc b/encryption/encryption.cc
@@ -4,11 +4,11 @@
 #ifdef OPENSSL
 #include "encryption/encryption.h"
 
+#include <openssl/opensslv.h>
+
 #include <algorithm>
 #include <limits>
 
-#include <openssl/opensslv.h>
-
 #include "port/port.h"
 
 namespace rocksdb {
@@ -372,9 +372,14 @@ Status KeyManagedEncryptedEnv::ReuseWritableFile(
       s = Status::InvalidArgument("Unsupported encryption method: " +
                                   ToString(static_cast<int>(file_info.method)));
   }
-  if (s.ok()) {
-    s = key_manager_->RenameFile(old_fname, fname);
+  if (!s.ok()) {
+    return s;
+  }
+  s = key_manager_->LinkFile(old_fname, fname);
+  if (!s.ok()) {
+    return s;
   }
+  s = key_manager_->DeleteFile(old_fname);
   return s;
 }
 
@@ -426,22 +431,28 @@ Status KeyManagedEncryptedEnv::LinkFile(const std::string& src_fname,
   }
   s = target()->LinkFile(src_fname, dst_fname);
   if (!s.ok()) {
-    // Ignore error
-    key_manager_->DeleteFile(dst_fname);
+    Status delete_status __attribute__((__unused__)) =
+        key_manager_->DeleteFile(dst_fname);
+    assert(delete_status.ok());
   }
   return s;
 }
 
 Status KeyManagedEncryptedEnv::RenameFile(const std::string& src_fname,
                                           const std::string& dst_fname) {
-  Status s = key_manager_->RenameFile(src_fname, dst_fname);
+  // Link(copy)File instead of RenameFile to avoid losing src_fname info when
+  // failed to rename the src_fname in the file system.
+  Status s = key_manager_->LinkFile(src_fname, dst_fname);
   if (!s.ok()) {
     return s;
   }
   s = target()->RenameFile(src_fname, dst_fname);
-  if (!s.ok()) {
-    // Ignore error
-    key_manager_->RenameFile(dst_fname, src_fname);
+  if (s.ok()) {
+    s = key_manager_->DeleteFile(src_fname);
+  } else {
+    Status delete_status __attribute__((__unused__)) =
+        key_manager_->DeleteFile(dst_fname);
+    assert(delete_status.ok());
   }
   return s;
 }

diff --git a/encryption/in_memory_key_manager.h b/encryption/in_memory_key_manager.h
@@ -71,17 +71,6 @@ class InMemoryKeyManager final : public KeyManager {
     return Status::OK();
   }
 
-  Status RenameFile(const std::string& src_fname,
-                    const std::string& dst_fname) override {
-    MutexLock l(&mu_);
-    if (files_.count(src_fname) == 0) {
-      return Status::Corruption("File not found: " + src_fname);
-    }
-    files_[dst_fname] = files_[src_fname];
-    files_.erase(src_fname);
-    return Status::OK();
-  }
-
  private:
   mutable port::Mutex mu_;
   Random rnd_;

diff --git a/include/rocksdb/encryption.h b/include/rocksdb/encryption.h
@@ -55,8 +55,6 @@ class KeyManager {
   virtual Status DeleteFile(const std::string& fname) = 0;
   virtual Status LinkFile(const std::string& src_fname,
                           const std::string& dst_fname) = 0;
-  virtual Status RenameFile(const std::string& src_fname,
-                            const std::string& dst_fname) = 0;
 };
 
 // An Env with underlying files being encrypted. It holds a reference to an