You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2016/04/20 02:37:48 UTC
[3/3] incubator-kudu git commit: KUDU-1377 (part 1): Add
Env::NewTempRWFile()
KUDU-1377 (part 1): Add Env::NewTempRWFile()
Previously, temp files were only supported for WritableFiles. Add
support for RWFiles.
Also make reading from a RWFile or RandomAccessFile retry silently on
EINTR.
Change-Id: I4f095f5a59607248040020ff0b113ba2ffb35aa2
Reviewed-on: http://gerrit.cloudera.org:8080/2814
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/832030a8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/832030a8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/832030a8
Branch: refs/heads/master
Commit: 832030a89b6b17330744ae20e29d2eaff643e403
Parents: 75d5c82
Author: Mike Percy <mp...@apache.org>
Authored: Mon Apr 18 00:44:58 2016 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Tue Apr 19 22:09:23 2016 +0000
----------------------------------------------------------------------
src/kudu/util/env-test.cc | 13 ++++++++++
src/kudu/util/env.h | 13 ++++++++--
src/kudu/util/env_posix.cc | 48 ++++++++++++++++++++++++++-----------
src/kudu/util/memenv/memenv.cc | 22 +++++++++++++----
4 files changed, 75 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/832030a8/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 5079542..50df0e2 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -740,4 +740,17 @@ TEST_F(TestEnv, TestCopyFile) {
NO_FATALS(ReadAndVerifyTestData(copy.get(), 0, kFileSize));
}
+// Simple regression test for NewTempRWFile().
+TEST_F(TestEnv, TestTempRWFile) {
+ string tmpl = "foo.XXXXXX";
+ string path;
+ gscoped_ptr<RWFile> file;
+
+ ASSERT_OK(env_->NewTempRWFile(RWFileOptions(), tmpl, &path, &file));
+ ASSERT_NE(path, tmpl);
+ ASSERT_EQ(0, path.find("foo."));
+ ASSERT_OK(file->Close());
+ ASSERT_OK(env_->DeleteFile(path));
+}
+
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/832030a8/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 5b3ff77..0283f5a 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -127,6 +127,10 @@ class Env {
const std::string& fname,
gscoped_ptr<RWFile>* result) = 0;
+ // Same as abovoe for NewTempWritableFile(), but for an RWFile.
+ virtual Status NewTempRWFile(const RWFileOptions& opts, const std::string& name_template,
+ std::string* created_filename, gscoped_ptr<RWFile>* res) = 0;
+
// Returns true iff the named file exists.
virtual bool FileExists(const std::string& fname) = 0;
@@ -429,8 +433,9 @@ class RWFile {
// 'scratch[0..length-1]', which must be live when '*result' is used.
// If an error was encountered, returns a non-OK status.
//
- // In the event of a "short read" (fewer bytes read than were requested),
- // an IOError is returned.
+ // This method will internally retry on EINTR and "short reads" in order to
+ // fully read the requested number of bytes. In the event that it is not
+ // possible to read exactly 'length' bytes, an IOError is returned.
//
// Safe for concurrent use by multiple threads.
virtual Status Read(uint64_t offset, size_t length,
@@ -556,6 +561,10 @@ class EnvWrapper : public Env {
gscoped_ptr<RWFile>* r) OVERRIDE {
return target_->NewRWFile(o, f, r);
}
+ Status NewTempRWFile(const RWFileOptions& o, const std::string& t,
+ std::string* f, gscoped_ptr<RWFile>* r) OVERRIDE {
+ return target_->NewTempRWFile(o, t, f, r);
+ }
bool FileExists(const std::string& f) OVERRIDE { return target_->FileExists(f); }
Status GetChildren(const std::string& dir, std::vector<std::string>* r) OVERRIDE {
return target_->GetChildren(dir, r);
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/832030a8/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index fe964fa..73399ca 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -262,12 +262,13 @@ class PosixRandomAccessFile: public RandomAccessFile {
uint8_t *scratch) const OVERRIDE {
ThreadRestrictions::AssertIOAllowed();
Status s;
- ssize_t r = pread(fd_, scratch, n, static_cast<off_t>(offset));
- *result = Slice(scratch, (r < 0) ? 0 : r);
+ ssize_t r;
+ RETRY_ON_EINTR(r, pread(fd_, scratch, n, offset));
if (r < 0) {
// An error: return a non-ok status.
s = IOError(filename_, errno);
}
+ *result = Slice(scratch, r);
return s;
}
@@ -486,7 +487,6 @@ class PosixWritableFile : public WritableFile {
};
class PosixRWFile : public RWFile {
-// is not employed.
public:
PosixRWFile(string fname, int fd, bool sync_on_close)
: filename_(std::move(fname)),
@@ -506,7 +506,8 @@ class PosixRWFile : public RWFile {
int rem = length;
uint8_t* dst = scratch;
while (rem > 0) {
- ssize_t r = pread(fd_, dst, rem, offset);
+ ssize_t r;
+ RETRY_ON_EINTR(r, pread(fd_, dst, rem, offset));
if (r < 0) {
// An error: return a non-ok status.
return IOError(filename_, errno);
@@ -739,16 +740,12 @@ class PosixEnv : public Env {
std::string* created_filename,
gscoped_ptr<WritableFile>* result) OVERRIDE {
TRACE_EVENT1("io", "PosixEnv::NewTempWritableFile", "template", name_template);
- ThreadRestrictions::AssertIOAllowed();
- gscoped_ptr<char[]> fname(new char[name_template.size() + 1]);
- ::snprintf(fname.get(), name_template.size() + 1, "%s", name_template.c_str());
- const int fd = ::mkstemp(fname.get());
- if (fd < 0) {
- return IOError(Substitute("Call to mkstemp() failed on name template $0", name_template),
- errno);
- }
- *created_filename = fname.get();
- return InstantiateNewWritableFile(*created_filename, fd, opts, result);
+ int fd;
+ string tmp_filename;
+ RETURN_NOT_OK(MkTmpFile(name_template, &fd, &tmp_filename));
+ RETURN_NOT_OK(InstantiateNewWritableFile(tmp_filename, fd, opts, result));
+ created_filename->swap(tmp_filename);
+ return Status::OK();
}
virtual Status NewRWFile(const string& fname,
@@ -766,6 +763,15 @@ class PosixEnv : public Env {
return Status::OK();
}
+ virtual Status NewTempRWFile(const RWFileOptions& opts, const std::string& name_template,
+ std::string* created_filename, gscoped_ptr<RWFile>* res) OVERRIDE {
+ TRACE_EVENT1("io", "PosixEnv::NewTempRWFile", "template", name_template);
+ int fd;
+ RETURN_NOT_OK(MkTmpFile(name_template, &fd, created_filename));
+ res->reset(new PosixRWFile(*created_filename, fd, opts.sync_on_close));
+ return Status::OK();
+ }
+
virtual bool FileExists(const std::string& fname) OVERRIDE {
TRACE_EVENT1("io", "PosixEnv::FileExists", "path", fname);
ThreadRestrictions::AssertIOAllowed();
@@ -1114,6 +1120,20 @@ class PosixEnv : public Env {
}
};
+ Status MkTmpFile(const string& name_template, int* fd, string* created_filename) {
+ ThreadRestrictions::AssertIOAllowed();
+ gscoped_ptr<char[]> fname(new char[name_template.size() + 1]);
+ ::snprintf(fname.get(), name_template.size() + 1, "%s", name_template.c_str());
+ int created_fd = mkstemp(fname.get());
+ if (created_fd < 0) {
+ return IOError(Substitute("Call to mkstemp() failed on name template $0", name_template),
+ errno);
+ }
+ *fd = created_fd;
+ *created_filename = fname.get();
+ return Status::OK();
+ }
+
Status InstantiateNewWritableFile(const std::string& fname,
int fd,
const WritableFileOptions& opts,
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/832030a8/src/kudu/util/memenv/memenv.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/memenv/memenv.cc b/src/kudu/util/memenv/memenv.cc
index 1af79df..3cc41ed 100644
--- a/src/kudu/util/memenv/memenv.cc
+++ b/src/kudu/util/memenv/memenv.cc
@@ -410,10 +410,9 @@ class InMemoryEnv : public EnvWrapper {
return NewRWFile(RWFileOptions(), fname, result);
}
- virtual Status NewTempWritableFile(const WritableFileOptions& opts,
- const std::string& name_template,
- std::string* created_filename,
- gscoped_ptr<WritableFile>* result) OVERRIDE {
+ template <typename WritableFileType, typename WritableFileImplType>
+ Status NewTmpFile(const string& name_template, string* created_filename,
+ gscoped_ptr<WritableFileType>* result) {
// Not very random, but InMemoryEnv is basically a test env.
Random random(GetCurrentTimeMicros());
while (true) {
@@ -427,7 +426,8 @@ class InMemoryEnv : public EnvWrapper {
MutexLock lock(mutex_);
if (!ContainsKey(file_map_, path)) {
- CreateAndRegisterNewWritableFileUnlocked<WritableFile, WritableFileImpl>(path, result);
+ CreateAndRegisterNewWritableFileUnlocked<WritableFileType, WritableFileImplType>(
+ path, result);
*created_filename = path;
return Status::OK();
}
@@ -435,6 +435,18 @@ class InMemoryEnv : public EnvWrapper {
// Unreachable.
}
+ virtual Status NewTempWritableFile(const WritableFileOptions& opts,
+ const std::string& name_template,
+ std::string* created_filename,
+ gscoped_ptr<WritableFile>* result) OVERRIDE {
+ return NewTmpFile<WritableFile, WritableFileImpl>(name_template, created_filename, result);
+ }
+
+ virtual Status NewTempRWFile(const RWFileOptions& opts, const std::string& name_template,
+ std::string* created_filename, gscoped_ptr<RWFile>* res) OVERRIDE {
+ return NewTmpFile<RWFile, RWFileImpl>(name_template, created_filename, res);
+ }
+
virtual bool FileExists(const std::string& fname) OVERRIDE {
MutexLock lock(mutex_);
return file_map_.find(fname) != file_map_.end();