kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: KUDU-1634 (part 3). Changed tmp files' infix from ".tmp" to ".kudutmp"
Date Wed, 30 Nov 2016 03:45:26 GMT
Repository: kudu
Updated Branches:
  refs/heads/master ca3b162e1 -> 8ff4d1211


KUDU-1634 (part 3). Changed tmp files' infix from ".tmp" to ".kudutmp"

As these ".tmp" strings were scattered over the code, I've tried to remove
this duplication as well by defining the single kTmpInfix in path_util.h.
In a couple of places it led to string concatenations, but I suppose it's
not that bad for overall performance?
The necessity of moving from "tmp" to "kudutmp" is backed by the idea that
we don't want to remove any user's data accidentally. For example, we may
have a symlink to some external directory, which doesn't belong only to
Kudu. Hence, we don't want to delete any ordinary tmp files from it.
(see https://gerrit.cloudera.org/#/c/5007/).

Change-Id: I52a794cc16d008570039d276ab32e6a26829bc53
Reviewed-on: http://gerrit.cloudera.org:8080/5123
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <adar@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8ff4d121
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8ff4d121
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8ff4d121

Branch: refs/heads/master
Commit: 8ff4d1211d2a9f20203774bd263112ec0d7333e1
Parents: ca3b162
Author: Maxim Smyatkin <smyatkinmaxim@gmail.com>
Authored: Thu Nov 17 11:22:38 2016 +0300
Committer: Adar Dembo <adar@cloudera.com>
Committed: Wed Nov 30 03:42:39 2016 +0000

----------------------------------------------------------------------
 src/kudu/consensus/log.cc                           |  5 ++---
 src/kudu/fs/fs_manager-test.cc                      | 16 ++++++++--------
 src/kudu/fs/fs_manager.cc                           |  4 +---
 src/kudu/fs/fs_manager.h                            |  1 -
 .../external_mini_cluster_fs_inspector.cc           |  2 +-
 src/kudu/tserver/tablet_copy_client.cc              |  4 ++--
 src/kudu/tserver/tablet_copy_source_session-test.cc |  4 +++-
 src/kudu/util/path_util.cc                          |  2 ++
 src/kudu/util/path_util.h                           |  3 +++
 src/kudu/util/pb_util.cc                            |  6 +++---
 10 files changed, 25 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/consensus/log.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc
index 78a8453..3fee204 100644
--- a/src/kudu/consensus/log.cc
+++ b/src/kudu/consensus/log.cc
@@ -125,8 +125,6 @@ static bool ValidateLogsToRetain(const char* flagname, int value) {
 static bool dummy = google::RegisterFlagValidator(
     &FLAGS_log_min_segments_to_retain, &ValidateLogsToRetain);
 
-static const char kSegmentPlaceholderFileTemplate[] = ".tmp.newsegmentXXXXXX";
-
 namespace kudu {
 namespace log {
 
@@ -969,7 +967,8 @@ Status Log::ReplaceSegmentInReaderUnlocked() {
 Status Log::CreatePlaceholderSegment(const WritableFileOptions& opts,
                                      string* result_path,
                                      shared_ptr<WritableFile>* out) {
-  string path_tmpl = JoinPathSegments(log_dir_, kSegmentPlaceholderFileTemplate);
+  string tmp_suffix = strings::Substitute("$0$1", kTmpInfix, ".newsegmentXXXXXX");
+  string path_tmpl = JoinPathSegments(log_dir_, tmp_suffix);
   VLOG(2) << "Creating temp. file for place holder segment, template: " << path_tmpl;
   unique_ptr<WritableFile> segment_file;
   RETURN_NOT_OK(fs_manager_->env()->NewTempWritableFile(opts,

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/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 1bc3c0d..dcaf286 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -137,9 +137,9 @@ TEST_F(FsManagerTestBase, TestListTablets) {
   string path = fs_manager()->GetTabletMetadataDir();
   unique_ptr<WritableFile> writer;
   ASSERT_OK(env_->NewWritableFile(
-      JoinPathSegments(path, "foo.tmp"), &writer));
+      JoinPathSegments(path, "foo.kudutmp"), &writer));
   ASSERT_OK(env_->NewWritableFile(
-      JoinPathSegments(path, "foo.tmp.abc123"), &writer));
+      JoinPathSegments(path, "foo.kudutmp.abc123"), &writer));
   ASSERT_OK(env_->NewWritableFile(
       JoinPathSegments(path, ".hidden"), &writer));
   ASSERT_OK(env_->NewWritableFile(
@@ -218,7 +218,7 @@ Status CountTmpFiles(Env* env, const string& path, const vector<string>&
childre
         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) {
+    } else if (name.find(kTmpInfix) != string::npos) {
       (*count)++;
     }
   }
@@ -244,20 +244,20 @@ TEST_F(FsManagerTestBase, TestTmpFilesCleanup) {
   // Create a few tmp files here
   shared_ptr<WritableFile> tmp_writer;
 
-  string tmp_path = JoinPathSegments(fs_manager()->GetWalsRootDir(), "wal.tmp.file");
+  string tmp_path = JoinPathSegments(fs_manager()->GetWalsRootDir(), "wal.kudutmp.file");
   ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer));
 
-  tmp_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[0], "data1.tmp.file");
+  tmp_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[0], "data1.kudutmp.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");
+  // Not a misprint here: checking for just ".kudutmp" as well
+  tmp_path = JoinPathSegments(fs_manager()->GetDataRootDirs()[1], "data2.kudutmp");
   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");
+  tmp_path = JoinPathSegments(nested_dir_path, "data4.kudutmp.file");
   ASSERT_OK(env_util::OpenFileForWrite(fs_manager()->env(), tmp_path, &tmp_writer));
 
   // Add a loop using symlink

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 55c78dc..c907ea0 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -102,8 +102,6 @@ const char *FsManager::kCorruptedSuffix = ".corrupted";
 const char *FsManager::kInstanceMetadataFileName = "instance";
 const char *FsManager::kConsensusMetadataDirName = "consensus-meta";
 
-const char *FsManager::kTmpInfix = ".tmp";
-
 FsManagerOpts::FsManagerOpts()
   : wal_path(FLAGS_fs_wal_dir),
     read_only(false) {
@@ -410,7 +408,7 @@ string FsManager::GetTabletMetadataPath(const string& tablet_id) const
{
 namespace {
 // Return true if 'fname' is a valid tablet ID.
 bool IsValidTabletId(const string& fname) {
-  if (fname.find(FsManager::kTmpInfix) != string::npos) {
+  if (fname.find(kTmpInfix) != string::npos) {
     LOG(WARNING) << "Ignoring tmp file in tablet metadata dir: " << fname;
     return false;
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/fs/fs_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index 8af725d..b86edf1 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -96,7 +96,6 @@ 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);

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
index 2fa40b3..b9667f0 100644
--- a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
+++ b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
@@ -59,7 +59,7 @@ Status ExternalMiniClusterFsInspector::ListFilesInDir(const string&
path,
   RETURN_NOT_OK(env_->GetChildren(path, entries));
   auto iter = entries->begin();
   while (iter != entries->end()) {
-    if (*iter == "." || *iter == ".." || iter->find(".tmp.") != string::npos) {
+    if (*iter == "." || *iter == ".." || iter->find(kTmpInfix) != string::npos) {
       iter = entries->erase(iter);
       continue;
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/tserver/tablet_copy_client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_client.cc b/src/kudu/tserver/tablet_copy_client.cc
index f0907b5..72fd7c9 100644
--- a/src/kudu/tserver/tablet_copy_client.cc
+++ b/src/kudu/tserver/tablet_copy_client.cc
@@ -277,7 +277,7 @@ Status TabletCopyClient::Finish() {
 
   if (FLAGS_tablet_copy_save_downloaded_metadata) {
     string meta_path = fs_manager_->GetTabletMetadataPath(tablet_id_);
-    string meta_copy_path = Substitute("$0.copy.$1.tmp", meta_path, start_time_micros_);
+    string meta_copy_path = Substitute("$0.copy.$1$2", meta_path, start_time_micros_, kTmpInfix);
     RETURN_NOT_OK_PREPEND(CopyFile(Env::Default(), meta_path, meta_copy_path,
                                    WritableFileOptions()),
                           "Unable to make copy of tablet metadata");
@@ -452,7 +452,7 @@ Status TabletCopyClient::WriteConsensusMetadata() {
 
   if (FLAGS_tablet_copy_save_downloaded_metadata) {
     string cmeta_path = fs_manager_->GetConsensusMetadataPath(tablet_id_);
-    string cmeta_copy_path = Substitute("$0.copy.$1.tmp", cmeta_path, start_time_micros_);
+    string cmeta_copy_path = Substitute("$0.copy.$1$2", cmeta_path, start_time_micros_, kTmpInfix);
     RETURN_NOT_OK_PREPEND(CopyFile(Env::Default(), cmeta_path, cmeta_copy_path,
                                    WritableFileOptions()),
                           "Unable to make copy of consensus metadata");

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/tserver/tablet_copy_source_session-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_source_session-test.cc b/src/kudu/tserver/tablet_copy_source_session-test.cc
index 9be1c81..522ce6f 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -199,7 +199,9 @@ class TabletCopyTest : public KuduTabletTest {
 
     // Write the file to a temporary location.
     WritableFileOptions opts;
-    string path_template = GetTestPath(Substitute("test_block_$0.tmp.XXXXXX", block_id.ToString()));
+    string path_template = GetTestPath(Substitute("test_block_$0$1.XXXXXX",
+                                                  block_id.ToString(),
+                                                  kTmpInfix));
     unique_ptr<WritableFile> writable_file;
     CHECK_OK(Env::Default()->NewTempWritableFile(opts, path_template, path, &writable_file));
     CHECK_OK(writable_file->Append(Slice(data.data(), data.size())));

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/util/path_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/path_util.cc b/src/kudu/util/path_util.cc
index 7131431..2973740 100644
--- a/src/kudu/util/path_util.cc
+++ b/src/kudu/util/path_util.cc
@@ -33,6 +33,8 @@ using std::string;
 
 namespace kudu {
 
+const char kTmpInfix[] = ".kudutmp";
+
 std::string JoinPathSegments(const std::string &a,
                              const std::string &b) {
   CHECK(!a.empty()) << "empty first component: " << a;

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/util/path_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/path_util.h b/src/kudu/util/path_util.h
index a5307d2..218ab81 100644
--- a/src/kudu/util/path_util.h
+++ b/src/kudu/util/path_util.h
@@ -23,6 +23,9 @@
 
 namespace kudu {
 
+// Common tmp infix
+extern const char kTmpInfix[];
+
 // Join two path segments with the appropriate path separator,
 // if necessary.
 std::string JoinPathSegments(const std::string &a,

http://git-wip-us.apache.org/repos/asf/kudu/blob/8ff4d121/src/kudu/util/pb_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index cbf71dd..8d154d3 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -105,7 +105,7 @@ std::ostream& operator<< (std::ostream& os, const kudu::pb_util::FileState&
stat
 namespace kudu {
 namespace pb_util {
 
-static const char* const kTmpTemplateSuffix = ".tmp.XXXXXX";
+static const char* const kTmpTemplateSuffix = ".XXXXXX";
 
 // Protobuf container constants.
 static const uint32_t kPBContainerInvalidVersion = 0;
@@ -462,7 +462,7 @@ Status ParseFromArray(MessageLite* msg, const uint8_t* data, uint32_t
length) {
 Status WritePBToPath(Env* env, const std::string& path,
                      const MessageLite& msg,
                      SyncMode sync) {
-  const string tmp_template = path + kTmpTemplateSuffix;
+  const string tmp_template = path + kTmpInfix + kTmpTemplateSuffix;
   string tmp_path;
 
   unique_ptr<WritableFile> file;
@@ -872,7 +872,7 @@ Status WritePBContainerToPath(Env* env, const std::string& path,
     return Status::AlreadyPresent(Substitute("File $0 already exists", path));
   }
 
-  const string tmp_template = path + kTmpTemplateSuffix;
+  const string tmp_template = path + kTmpInfix + kTmpTemplateSuffix;
   string tmp_path;
 
   unique_ptr<RWFile> file;


Mime
View raw message