You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2018/08/15 19:03:21 UTC

[1/2] kudu git commit: [build] Switch to sha512 checksum for releases

Repository: kudu
Updated Branches:
  refs/heads/master 8654a2115 -> dfa465265


[build] Switch to sha512 checksum for releases

According to the current release distribution guide[1] the checksum file
SHOULD be SHA-256 and/or SHA-512 and SHOULD NOT be SHA-1

SHA-512 seemed more future-proof than SHA-256

Another change is the separator between the checksum and the filename.
Previously it was a <TAB> character, but at least on MacOS the tab
version doesn't work when trying to verify the checksum with `shasum -c
<checksum_file>`

This commit changes it to double-space which works on both Linux using
`sha512sum` and MacOS using `shasum`.

Looking at a checksum created by `sha512sum` on Linux, it also uses
double-space as the separator, so this seems to be the de-facto
standard:

$ sha512sum apache-kudu-1.8.0-SNAPSHOT.tar.gz | xxd
0000070: 3065 3132 3666 3463 3664 6332 3065 6338  0e126f4c6dc20ec8
0000080: 2020 6170 6163 6865 2d6b 7564 752d 312e    apache-kudu-1.

(0x20 is <SPACE>[2])

[1] http://www.apache.org/dev/release-distribution#sigs-and-sums
[2] man 7 ascii

Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Reviewed-on: http://gerrit.cloudera.org:8080/11217
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: 08637705ae6ad8d3426cb7f8ed016cc1c0fbea46
Parents: 8654a21
Author: Attila Bukor <ab...@apache.org>
Authored: Tue Aug 14 21:56:58 2018 +0200
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Aug 15 18:49:07 2018 +0000

----------------------------------------------------------------------
 build-support/build_source_release.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/08637705/build-support/build_source_release.py
----------------------------------------------------------------------
diff --git a/build-support/build_source_release.py b/build-support/build_source_release.py
index 5ef39b2..5bf6950 100755
--- a/build-support/build_source_release.py
+++ b/build-support/build_source_release.py
@@ -125,13 +125,13 @@ def gen_sha_file(tarball_path):
   """
   Create a sha checksum file of the tarball.
 
-  The output format is compatible with command line tools like 'sha1sum' so it
+  The output format is compatible with command line tools like 'sha512sum' so it
   can be used to verify the checksum.
   """
-  digest = checksum_file(hashlib.sha1(), tarball_path)
-  path = tarball_path + ".sha1"
+  digest = checksum_file(hashlib.sha512(), tarball_path)
+  path = tarball_path + ".sha512"
   with open(path, "w") as f:
-    f.write("%s\t%s\n" % (digest, os.path.basename(tarball_path)))
+      f.write("%s  %s\n" % (digest, os.path.basename(tarball_path)))
   print(Colors.GREEN + "Generated sha:\t\t" + Colors.RESET + path)
 
 


[2/2] kudu git commit: file_cache: stop using sync_on_close when reopening evicted files

Posted by ad...@apache.org.
file_cache: stop using sync_on_close when reopening evicted files

When the FileCache was originally written, PosixRWFile had a pending_sync_
member which tracked whether or not a file was dirty and needed to be
synced. However, with its removal in commit 9a07b2fed, sync_on_close=true
should no longer be necessary for correctness.

I only noticed this because of a "Time spent sync" log message emitted while
running 'kudu fs dump', which was perplexing seeing as this CLI invocation
is read-only. An unnecessary fdatasync system call should no-op, but maybe
that's not the case on older kernels?

While I was there, I cleaned up env_posix.cc a bit, including constifying
an fd_ member and replacing its "is the file closed?" semantic with a new
dedicated boolean.

Change-Id: I845f39ea202ec0cb46ce8e841c6cfb6c3a8dad2c
Reviewed-on: http://gerrit.cloudera.org:8080/11190
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: dfa465265c44098fc741da173e150fae0253501f
Parents: 0863770
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Aug 10 13:21:13 2018 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Wed Aug 15 19:02:55 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/env.h         | 10 +++++
 src/kudu/util/env_posix.cc  | 86 ++++++++++++++++++++--------------------
 src/kudu/util/file_cache.cc |  4 --
 3 files changed, 54 insertions(+), 46 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/dfa46526/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 2822994..88c1c0d 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -400,6 +400,11 @@ class SequentialFile {
 };
 
 // A file abstraction for randomly reading the contents of a file.
+//
+// Note: this abstraction is safe to use in FileCache, which means all
+// implementations must ensure that any mutable state changes brought on by
+// instance destruction and recreation (i.e. triggered by cache eviction and
+// reloading events) do not affect correctness.
 class RandomAccessFile {
  public:
   RandomAccessFile() { }
@@ -535,6 +540,11 @@ struct RWFileOptions {
 // noted otherwise) bearing in mind the usual filesystem coherency guarantees
 // (e.g. two threads that write concurrently to the same file offset will
 // probably yield garbage).
+//
+// Note: this abstraction is safe to use in FileCache, which means all
+// implementations must ensure that any mutable state changes brought on by
+// instance destruction and recreation (i.e. triggered by cache eviction and
+// reloading events) do not affect correctness.
 class RWFile {
  public:
   enum FlushMode {

http://git-wip-us.apache.org/repos/asf/kudu/blob/dfa46526/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index fe47bfd..b6215f8 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -296,10 +296,10 @@ class ScopedFdCloser {
   }
 
  private:
-  int fd_;
+  const int fd_;
 };
 
-Status IOError(const std::string& context, int err_number) {
+Status IOError(const string& context, int err_number) {
   switch (err_number) {
     case ENOENT:
       return Status::NotFound(context, ErrnoToString(err_number), err_number);
@@ -544,11 +544,11 @@ const char* ResourceLimitTypeToMacosRlimit(Env::ResourceLimitType t) {
 
 class PosixSequentialFile: public SequentialFile {
  private:
-  std::string filename_;
-  FILE* file_;
+  const string filename_;
+  FILE* const file_;
 
  public:
-  PosixSequentialFile(std::string fname, FILE* f)
+  PosixSequentialFile(string fname, FILE* f)
       : filename_(std::move(fname)), file_(f) {}
   virtual ~PosixSequentialFile() {
     int err;
@@ -593,11 +593,11 @@ class PosixSequentialFile: public SequentialFile {
 // pread() based random-access
 class PosixRandomAccessFile: public RandomAccessFile {
  private:
-  std::string filename_;
-  int fd_;
+  const string filename_;
+  const int fd_;
 
  public:
-  PosixRandomAccessFile(std::string fname, int fd)
+  PosixRandomAccessFile(string fname, int fd)
       : filename_(std::move(fname)), fd_(fd) {}
   virtual ~PosixRandomAccessFile() {
     int err;
@@ -640,19 +640,18 @@ class PosixRandomAccessFile: public RandomAccessFile {
 // order to further improve Sync() performance.
 class PosixWritableFile : public WritableFile {
  public:
-  PosixWritableFile(std::string fname, int fd, uint64_t file_size,
+  PosixWritableFile(string fname, int fd, uint64_t file_size,
                     bool sync_on_close)
       : filename_(std::move(fname)),
         fd_(fd),
         sync_on_close_(sync_on_close),
         filesize_(file_size),
         pre_allocated_size_(0),
-        pending_sync_(false) {}
+        pending_sync_(false),
+        closed_(false) {}
 
   ~PosixWritableFile() {
-    if (fd_ >= 0) {
-      WARN_NOT_OK(Close(), "Failed to close " + filename_);
-    }
+    WARN_NOT_OK(Close(), "Failed to close " + filename_);
   }
 
   virtual Status Append(const Slice& data) OVERRIDE {
@@ -694,6 +693,9 @@ class PosixWritableFile : public WritableFile {
   }
 
   virtual Status Close() OVERRIDE {
+    if (closed_) {
+      return Status::OK();
+    }
     TRACE_EVENT1("io", "PosixWritableFile::Close", "path", filename_);
     ThreadRestrictions::AssertIOAllowed();
     MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO));
@@ -728,7 +730,7 @@ class PosixWritableFile : public WritableFile {
       }
     }
 
-    fd_ = -1;
+    closed_ = true;
     return s;
   }
 
@@ -772,13 +774,14 @@ class PosixWritableFile : public WritableFile {
   virtual const string& filename() const OVERRIDE { return filename_; }
 
  private:
-  const std::string filename_;
-  int fd_;
-  bool sync_on_close_;
+  const string filename_;
+  const int fd_;
+  const bool sync_on_close_;
+
   uint64_t filesize_;
   uint64_t pre_allocated_size_;
-
   bool pending_sync_;
+  bool closed_;
 };
 
 class PosixRWFile : public RWFile {
@@ -1033,7 +1036,7 @@ class PosixRWFile : public RWFile {
     }
   }
 
-  const std::string filename_;
+  const string filename_;
   const int fd_;
   const bool sync_on_close_;
 
@@ -1069,7 +1072,7 @@ class PosixEnv : public Env {
     exit(1);
   }
 
-  virtual Status NewSequentialFile(const std::string& fname,
+  virtual Status NewSequentialFile(const string& fname,
                                    unique_ptr<SequentialFile>* result) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::NewSequentialFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
@@ -1083,13 +1086,13 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status NewRandomAccessFile(const std::string& fname,
+  virtual Status NewRandomAccessFile(const string& fname,
                                      unique_ptr<RandomAccessFile>* result) OVERRIDE {
     return NewRandomAccessFile(RandomAccessFileOptions(), fname, result);
   }
 
   virtual Status NewRandomAccessFile(const RandomAccessFileOptions& opts,
-                                     const std::string& fname,
+                                     const string& fname,
                                      unique_ptr<RandomAccessFile>* result) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::NewRandomAccessFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
@@ -1104,13 +1107,13 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status NewWritableFile(const std::string& fname,
+  virtual Status NewWritableFile(const string& fname,
                                  unique_ptr<WritableFile>* result) OVERRIDE {
     return NewWritableFile(WritableFileOptions(), fname, result);
   }
 
   virtual Status NewWritableFile(const WritableFileOptions& opts,
-                                 const std::string& fname,
+                                 const string& fname,
                                  unique_ptr<WritableFile>* result) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::NewWritableFile", "path", fname);
     int fd;
@@ -1119,8 +1122,8 @@ class PosixEnv : public Env {
   }
 
   virtual Status NewTempWritableFile(const WritableFileOptions& opts,
-                                     const std::string& name_template,
-                                     std::string* created_filename,
+                                     const string& name_template,
+                                     string* created_filename,
                                      unique_ptr<WritableFile>* result) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::NewTempWritableFile", "template", name_template);
     int fd;
@@ -1146,8 +1149,8 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status NewTempRWFile(const RWFileOptions& opts, const std::string& name_template,
-                               std::string* created_filename, unique_ptr<RWFile>* res) OVERRIDE {
+  virtual Status NewTempRWFile(const RWFileOptions& opts, const string& name_template,
+                               string* created_filename, unique_ptr<RWFile>* res) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::NewTempRWFile", "template", name_template);
     int fd;
     RETURN_NOT_OK(MkTmpFile(name_template, &fd, created_filename));
@@ -1155,14 +1158,13 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual bool FileExists(const std::string& fname) OVERRIDE {
+  virtual bool FileExists(const string& fname) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::FileExists", "path", fname);
     ThreadRestrictions::AssertIOAllowed();
     return access(fname.c_str(), F_OK) == 0;
   }
 
-  virtual Status GetChildren(const std::string& dir,
-                             std::vector<std::string>* result) OVERRIDE {
+  virtual Status GetChildren(const string& dir, vector<string>* result) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::GetChildren", "path", dir);
     MAYBE_RETURN_EIO(dir, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1180,7 +1182,7 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status DeleteFile(const std::string& fname) OVERRIDE {
+  virtual Status DeleteFile(const string& fname) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::DeleteFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1191,7 +1193,7 @@ class PosixEnv : public Env {
     return result;
   };
 
-  virtual Status CreateDir(const std::string& name) OVERRIDE {
+  virtual Status CreateDir(const string& name) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::CreateDir", "path", name);
     MAYBE_RETURN_EIO(name, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1202,7 +1204,7 @@ class PosixEnv : public Env {
     return result;
   };
 
-  virtual Status DeleteDir(const std::string& name) OVERRIDE {
+  virtual Status DeleteDir(const string& name) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::DeleteDir", "path", name);
     MAYBE_RETURN_EIO(name, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1237,7 +1239,7 @@ class PosixEnv : public Env {
     return result;
   }
 
-  virtual Status SyncDir(const std::string& dirname) OVERRIDE {
+  virtual Status SyncDir(const string& dirname) OVERRIDE {
     TRACE_EVENT1("io", "SyncDir", "path", dirname);
     MAYBE_RETURN_EIO(dirname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1254,12 +1256,12 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status DeleteRecursively(const std::string &name) OVERRIDE {
+  virtual Status DeleteRecursively(const string &name) OVERRIDE {
     return Walk(name, POST_ORDER, Bind(&PosixEnv::DeleteRecursivelyCb,
                                        Unretained(this)));
   }
 
-  virtual Status GetFileSize(const std::string& fname, uint64_t* size) OVERRIDE {
+  virtual Status GetFileSize(const string& fname, uint64_t* size) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::GetFileSize", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1273,7 +1275,7 @@ class PosixEnv : public Env {
     return s;
   }
 
-  virtual Status GetFileSizeOnDisk(const std::string& fname, uint64_t* size) OVERRIDE {
+  virtual Status GetFileSizeOnDisk(const string& fname, uint64_t* size) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::GetFileSizeOnDisk", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     ThreadRestrictions::AssertIOAllowed();
@@ -1355,7 +1357,7 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  virtual Status RenameFile(const std::string& src, const std::string& target) OVERRIDE {
+  virtual Status RenameFile(const string& src, const string& target) OVERRIDE {
     TRACE_EVENT2("io", "PosixEnv::RenameFile", "src", src, "dst", target);
     MAYBE_RETURN_EIO(src, IOError(Env::kInjectedFailureStatusMsg, EIO));
     MAYBE_RETURN_EIO(target, IOError(Env::kInjectedFailureStatusMsg, EIO));
@@ -1367,7 +1369,7 @@ class PosixEnv : public Env {
     return result;
   }
 
-  virtual Status LockFile(const std::string& fname, FileLock** lock) OVERRIDE {
+  virtual Status LockFile(const string& fname, FileLock** lock) OVERRIDE {
     TRACE_EVENT1("io", "PosixEnv::LockFile", "path", fname);
     MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO));
     if (ShouldInject(fname, FLAGS_env_inject_lock_failure_globs)) {
@@ -1411,7 +1413,7 @@ class PosixEnv : public Env {
     return result;
   }
 
-  virtual Status GetTestDirectory(std::string* result) OVERRIDE {
+  virtual Status GetTestDirectory(string* result) OVERRIDE {
     string dir;
     const char* env = getenv("TEST_TMPDIR");
     if (env && env[0] != '\0') {
@@ -1780,7 +1782,7 @@ class PosixEnv : public Env {
     return Status::OK();
   }
 
-  Status InstantiateNewWritableFile(const std::string& fname,
+  Status InstantiateNewWritableFile(const string& fname,
                                     int fd,
                                     const WritableFileOptions& opts,
                                     unique_ptr<WritableFile>* result) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/dfa46526/src/kudu/util/file_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache.cc b/src/kudu/util/file_cache.cc
index a1ab814..3f987d7 100644
--- a/src/kudu/util/file_cache.cc
+++ b/src/kudu/util/file_cache.cc
@@ -331,11 +331,7 @@ class Descriptor<RWFile> : public RWFile {
     }
 
     // The file was evicted, reopen it.
-    //
-    // Because the file may be evicted at any time we must use 'sync_on_close'
-    // (note: sync is a no-op if the file isn't dirty).
     RWFileOptions opts;
-    opts.sync_on_close = true;
     opts.mode = Env::OPEN_EXISTING;
     unique_ptr<RWFile> f;
     RETURN_NOT_OK(base_.env()->NewRWFile(opts, base_.filename(), &f));