Skip to content

Commit

Permalink
Atomize RenameFile in KeyManagedEncryptedEnv (facebook#7)
Browse files Browse the repository at this point in the history
apache/incubator-pegasus#1575

Cherry-pick from
tikv@93e89a5

fix bug: tikv/tikv#9115

Summary: we need to update encryption metadata via
encryption::DataKeyManager, which cannot combine with the actual file
operation into one atomic operation. In RenameFile, when the src_file
has been removed, power is off, then we may lost the file info of
src_file next restart.

Signed-off-by: Xintao [hunterlxt@live.com](mailto:hunterlxt@live.com)

Signed-off-by: Xintao <hunterlxt@live.com>
Co-authored-by: Xintao <hunterlxt@live.com>
  • Loading branch information
acelyc111 and hunterlxt committed Sep 15, 2023
1 parent c5ac8d5 commit 2a2181e
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 24 deletions.
3 changes: 0 additions & 3 deletions db/db_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 19 additions & 8 deletions encryption/encryption.cc
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,14 @@ Status KeyManagedEncryptedEnv::ReuseWritableFile(
"Unsupported encryption method: " +
std::to_string(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;
}

Expand Down Expand Up @@ -437,22 +442,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;
}
Expand Down
11 changes: 0 additions & 11 deletions encryption/in_memory_key_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,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_;
Expand Down
2 changes: 0 additions & 2 deletions include/rocksdb/encryption.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2a2181e

Please sign in to comment.