Repository: kudu Updated Branches: refs/heads/master 1bee345a1 -> 85404f67b
KUDU-1634. TS and master should delete tmp metadata files on startup Initially it was suggested that TS should delete tmp files from consensus-meta directory, but these leftovers can appear anywhere where atomic write with WritePBContainerToPath() is being used. For now, it is consensus metadata, block manager's data directory, instance metadata, etc. Also, it happens to Master server as well. Hence, I've decided to deal with it on FsManager level and scan all root paths for tmp files. It may slow startup a little bit, but I don't know any way around it because: a) it can't be done on background as it can remove the real tmp files this way; b) it would be nice to have a separate tmp directory, but it makes rename not atomic this way (because we can - and usually will - have root/wal dirs on different drives). Change-Id: I299e7b22c3f920430ff5d581cf7a507a83eb0101 Reviewed-on: http://gerrit.cloudera.org:8080/5007 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/85404f67 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/85404f67 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/85404f67 Branch: refs/heads/master Commit: 85404f67bb7b25f0e2f4adc4b1903975d9c4aa5a Parents: 1bee345 Author: Maxim Smyatkin <[email protected]> Authored: Tue Nov 8 23:16:27 2016 +0300 Committer: Adar Dembo <[email protected]> Committed: Thu Nov 17 19:31:52 2016 +0000 ---------------------------------------------------------------------- src/kudu/fs/fs_manager-test.cc | 82 +++++++++++++++++++++++++++++++++++++ src/kudu/fs/fs_manager.cc | 73 ++++++++++++++++++++++++++++++--- src/kudu/fs/fs_manager.h | 5 +++ 3 files changed, 154 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/85404f67/src/kudu/fs/fs_manager-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/fs/fs_manager-test.cc b/src/kudu/fs/fs_manager-test.cc index 8077a7b..20340ad 100644 --- a/src/kudu/fs/fs_manager-test.cc +++ b/src/kudu/fs/fs_manager-test.cc @@ -18,17 +18,21 @@ #include <glog/logging.h> #include <glog/stl_logging.h> #include <gtest/gtest.h> +#include <unistd.h> +#include <unordered_set> #include "kudu/fs/block_manager.h" #include "kudu/fs/fs_manager.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/gutil/strings/util.h" +#include "kudu/util/env_util.h" #include "kudu/util/metrics.h" #include "kudu/util/oid_generator.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" using std::shared_ptr; +using std::unordered_set; using strings::Substitute; namespace kudu { @@ -193,4 +197,82 @@ TEST_F(FsManagerTestBase, TestFormatWithSpecificUUID) { ASSERT_EQ(uuid, fs_manager()->uuid()); } +Status CountTmpFiles(Env* env, const string& path, const vector<string>& children, + unordered_set<string>* checked_dirs, size_t* count) { + vector<string> sub_objects; + for (const string& name : children) { + if (name == "." || name == "..") continue; + + string sub_path; + RETURN_NOT_OK(env->Canonicalize(JoinPathSegments(path, name), &sub_path)); + bool is_directory; + RETURN_NOT_OK(env->IsDirectory(sub_path, &is_directory)); + if (is_directory) { + if (!ContainsKey(*checked_dirs, sub_path)) { + checked_dirs->insert(sub_path); + RETURN_NOT_OK(env->GetChildren(sub_path, &sub_objects)); + RETURN_NOT_OK(CountTmpFiles(env, sub_path, sub_objects, checked_dirs, count)); + } + } else if (name.find(FsManager::kTmpInfix) != string::npos) { + (*count)++; + } + } + return Status::OK(); +} + +Status CountTmpFiles(Env* env, const vector<string>& roots, size_t* count) { + unordered_set<string> checked_dirs; + for (const string& root : roots) { + vector<string> children; + RETURN_NOT_OK(env->GetChildren(root, &children)); + RETURN_NOT_OK(CountTmpFiles(env, root, children, &checked_dirs, count)); + } + return Status::OK(); +} + +TEST_F(FsManagerTestBase, TestTmpFilesCleanup) { + string wal_path = GetTestPath("wals"); + vector<string> data_paths = { GetTestPath("data1"), GetTestPath("data2"), GetTestPath("data3") }; + ReinitFsManager(wal_path, data_paths); + ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout()); + + // Create a few tmp files here + shared_ptr<WritableFile> tmp_writer; + + string tmp_path = JoinPathSegments(fs_manager()->GetWalsRootDir(), "wal.tmp.file"); + ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer)); + + tmp_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[0], "data1.tmp.file"); + ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer)); + + // Not a misprint here: checking for just ".tmp" as well + tmp_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[1], "data2.tmp"); + ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer)); + + // Try with nested directory + string nested_dir_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[2], "data4"); + ASSERT_OK(env_util::CreateDirIfMissing(fs_manager()->env(), nested_dir_path)); + tmp_path = JoinPathSegments(nested_dir_path, "data4.tmp.file"); + ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer)); + + // Add a loop using symlink + string data3_link = JoinPathSegments(nested_dir_path, "data3-link"); + int symlink_error = symlink(fs_manager()->GetDataRootDirs()[2].c_str(), data3_link.c_str()); + ASSERT_EQ(0, symlink_error); + + vector<string> lookup_dirs = fs_manager()->GetDataRootDirs(); + lookup_dirs.push_back(fs_manager()->GetWalsRootDir()); + + size_t n_tmp_files = 0; + ASSERT_OK(CountTmpFiles(fs_manager()->env(), lookup_dirs, &n_tmp_files)); + ASSERT_EQ(4, n_tmp_files); + + // Opening fs_manager should remove tmp files + ASSERT_OK(fs_manager()->Open()); + + n_tmp_files = 0; + ASSERT_OK(CountTmpFiles(fs_manager()->env(), lookup_dirs, &n_tmp_files)); + ASSERT_EQ(0, n_tmp_files); +} + } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/85404f67/src/kudu/fs/fs_manager.cc ---------------------------------------------------------------------- diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc index 86e5d4f..55c78dc 100644 --- a/src/kudu/fs/fs_manager.cc +++ b/src/kudu/fs/fs_manager.cc @@ -20,6 +20,7 @@ #include <deque> #include <iostream> #include <map> +#include <stack> #include <unordered_set> #include <boost/optional.hpp> @@ -81,7 +82,10 @@ using kudu::fs::LogBlockManager; using kudu::fs::ReadableBlock; using kudu::fs::WritableBlock; using std::map; +using std::stack; +using std::string; using std::unordered_set; +using std::vector; using strings::Substitute; namespace kudu { @@ -98,7 +102,7 @@ const char *FsManager::kCorruptedSuffix = ".corrupted"; const char *FsManager::kInstanceMetadataFileName = "instance"; const char *FsManager::kConsensusMetadataDirName = "consensus-meta"; -static const char* const kTmpInfix = ".tmp"; +const char *FsManager::kTmpInfix = ".tmp"; FsManagerOpts::FsManagerOpts() : wal_path(FLAGS_fs_wal_dir), @@ -226,6 +230,12 @@ void FsManager::InitBlockManager() { Status FsManager::Open() { RETURN_NOT_OK(Init()); + + // Remove leftover tmp files + if (!read_only_) { + CleanTmpFiles(); + } + for (const string& root : canonicalized_all_fs_roots_) { gscoped_ptr<InstanceMetadataPB> pb(new InstanceMetadataPB); RETURN_NOT_OK(pb_util::ReadPBContainerFromPath(env_, GetInstanceMetadataPath(root), @@ -381,7 +391,7 @@ const string& FsManager::uuid() const { vector<string> FsManager::GetDataRootDirs() const { // Add the data subdirectory to each data root. - std::vector<std::string> data_paths; + vector<string> data_paths; for (const string& data_fs_root : canonicalized_data_fs_roots_) { data_paths.push_back(JoinPathSegments(data_fs_root, kDataDirName)); } @@ -399,8 +409,8 @@ string FsManager::GetTabletMetadataPath(const string& tablet_id) const { namespace { // Return true if 'fname' is a valid tablet ID. -bool IsValidTabletId(const std::string& fname) { - if (fname.find(kTmpInfix) != string::npos) { +bool IsValidTabletId(const string& fname) { + if (fname.find(FsManager::kTmpInfix) != string::npos) { LOG(WARNING) << "Ignoring tmp file in tablet metadata dir: " << fname; return false; } @@ -449,6 +459,57 @@ string FsManager::GetWalSegmentFileName(const string& tablet_id, StringPrintf("%09" PRIu64, sequence_number))); } +void FsManager::CleanTmpFiles() { + DCHECK(!read_only_); + string canonized_path; + vector<string> children; + unordered_set<string> checked_dirs; + stack<string> paths; + for (const string& root : canonicalized_all_fs_roots_) { + paths.push(root); + } + + while (!paths.empty()) { + string path = paths.top(); + paths.pop(); + + Status s = env_->GetChildren(path, &children); + if (s.ok()) { + for (const string& child : children) { + if (child == "." || child == "..") continue; + + // Canonicalize in case of symlinks + s = env_->Canonicalize(JoinPathSegments(path, child), &canonized_path); + if (!s.ok()) { + LOG(WARNING) << "Unable to get the real path: " << s.ToString(); + continue; + } + + bool is_directory; + s = env_->IsDirectory(canonized_path, &is_directory); + if (!s.ok()) { + LOG(WARNING) << "Unable to get information about file: " << s.ToString(); + continue; + } + + if (is_directory) { + // Check if we didn't handle this path yet + if (!ContainsKey(checked_dirs, canonized_path)) { + checked_dirs.insert(canonized_path); + paths.push(canonized_path); + } + } else if (child.find(kTmpInfix) != string::npos) { + s = env_->DeleteFile(canonized_path); + if (!s.ok()) { + LOG(WARNING) << "Unable to delete tmp file: " << s.ToString(); + } + } + } + } else { + LOG(WARNING) << "Unable to read directory: " << s.ToString(); + } + } +} // ========================================================================== // Dump/Debug utils @@ -460,7 +521,7 @@ void FsManager::DumpFileSystemTree(ostream& out) { for (const string& root : canonicalized_all_fs_roots_) { out << "File-System Root: " << root << std::endl; - std::vector<string> objects; + vector<string> objects; Status s = env_->GetChildren(root, &objects); if (!s.ok()) { LOG(ERROR) << "Unable to list the fs-tree: " << s.ToString(); @@ -476,7 +537,7 @@ void FsManager::DumpFileSystemTree(ostream& out, const string& prefix, for (const string& name : objects) { if (name == "." || name == "..") continue; - std::vector<string> sub_objects; + vector<string> sub_objects; string sub_path = JoinPathSegments(path, name); Status s = env_->GetChildren(sub_path, &sub_objects); if (s.ok()) { http://git-wip-us.apache.org/repos/asf/kudu/blob/85404f67/src/kudu/fs/fs_manager.h ---------------------------------------------------------------------- diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h index 3c4420a..8af725d 100644 --- a/src/kudu/fs/fs_manager.h +++ b/src/kudu/fs/fs_manager.h @@ -96,6 +96,7 @@ class FsManager { public: static const char *kWalFileNamePrefix; static const char *kWalsRecoveryDirSuffix; + static const char *kTmpInfix; // Only for unit tests. FsManager(Env* env, const std::string& root_path); @@ -238,6 +239,10 @@ class FsManager { const std::string& path, const std::vector<std::string>& objects); + // Deletes temporary files left from previous execution (e.g., after a crash). + // Logs warnings in case of errors. + void CleanTmpFiles(); + static const char *kDataDirName; static const char *kTabletMetadataDirName; static const char *kWalDirName;
