kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: Reduce arguments in file Read API
Date Tue, 02 May 2017 22:16:21 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 867fc91cb -> 986ade9b0


Reduce arguments in file Read API

Because reads are now always read fully or result in an error if
all bytes can’t be read, we can simplify the Read API to pass
only a Slice instead of a slice, scratch, and length.

This patch changes the file Read APIs and all similar API calls
that call it.

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


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

Branch: refs/heads/master
Commit: 986ade9b0fc5eed892a7e63399745aed2ca82e98
Parents: 867fc91
Author: Grant Henke <granthenke@gmail.com>
Authored: Sun Apr 30 22:04:32 2017 -0500
Committer: Adar Dembo <adar@cloudera.com>
Committed: Tue May 2 22:16:00 2017 +0000

----------------------------------------------------------------------
 src/kudu/cfile/cfile_reader.cc                  | 19 ++++-----
 src/kudu/consensus/log_util.cc                  | 33 +++++++--------
 src/kudu/fs/block_manager-stress-test.cc        |  4 +-
 src/kudu/fs/block_manager-test.cc               | 20 ++++-----
 src/kudu/fs/block_manager.h                     | 14 +++----
 src/kudu/fs/file_block_manager.cc               | 10 ++---
 src/kudu/fs/fs-test-util.h                      |  7 ++--
 src/kudu/fs/fs_manager-test.cc                  |  4 +-
 src/kudu/fs/log_block_manager-test.cc           |  8 ++--
 src/kudu/fs/log_block_manager.cc                | 22 ++++------
 src/kudu/tserver/tablet_copy-test-base.h        |  9 +---
 src/kudu/tserver/tablet_copy_client-test.cc     |  7 ++--
 src/kudu/tserver/tablet_copy_service-test.cc    |  4 +-
 .../tserver/tablet_copy_source_session-test.cc  |  7 ++--
 src/kudu/tserver/tablet_copy_source_session.cc  |  4 +-
 src/kudu/tserver/tablet_copy_source_session.h   |  8 ++--
 src/kudu/util/env-test.cc                       | 34 ++++++++-------
 src/kudu/util/env.cc                            |  4 +-
 src/kudu/util/env.h                             | 38 +++++++----------
 src/kudu/util/env_posix.cc                      | 44 ++++++++++----------
 src/kudu/util/env_util.cc                       |  4 +-
 src/kudu/util/file_cache-stress-test.cc         |  4 +-
 src/kudu/util/file_cache-test.cc                |  4 +-
 src/kudu/util/file_cache.cc                     | 10 ++---
 src/kudu/util/pb_util-internal.cc               |  8 +---
 src/kudu/util/pb_util-test.cc                   |  4 +-
 src/kudu/util/pb_util.cc                        | 22 +---------
 src/kudu/util/rolling_log.cc                    |  4 +-
 28 files changed, 155 insertions(+), 205 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/cfile/cfile_reader.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile_reader.cc b/src/kudu/cfile/cfile_reader.cc
index 8e814f8..5135dc2 100644
--- a/src/kudu/cfile/cfile_reader.cc
+++ b/src/kudu/cfile/cfile_reader.cc
@@ -133,10 +133,9 @@ Status CFileReader::ReadMagicAndLength(uint64_t offset, uint32_t *len) {
   TRACE_EVENT1("io", "CFileReader::ReadMagicAndLength",
                "cfile", ToString());
   uint8_t scratch[kMagicAndLengthSize];
-  Slice slice;
+  Slice slice(scratch, kMagicAndLengthSize);
 
-  RETURN_NOT_OK(block_->Read(offset, kMagicAndLengthSize,
-                             &slice, scratch));
+  RETURN_NOT_OK(block_->Read(offset, &slice));
 
   return ParseMagicAndLength(slice, &cfile_version_, len);
 }
@@ -190,10 +189,9 @@ Status CFileReader::ReadAndParseHeader() {
 
   // Now read the protobuf header.
   uint8_t header_space[header_size];
-  Slice header_slice;
+  Slice header_slice(header_space, header_size);
   header_.reset(new CFileHeaderPB());
-  RETURN_NOT_OK(block_->Read(kMagicAndLengthSize, header_size,
-                             &header_slice, header_space));
+  RETURN_NOT_OK(block_->Read(kMagicAndLengthSize, &header_slice));
   if (!header_->ParseFromArray(header_slice.data(), header_size)) {
     return Status::Corruption("Invalid cfile pb header");
   }
@@ -220,10 +218,9 @@ Status CFileReader::ReadAndParseFooter() {
   // Now read the protobuf footer.
   footer_.reset(new CFileFooterPB());
   uint8_t footer_space[footer_size];
-  Slice footer_slice;
+  Slice footer_slice(footer_space, footer_size);
   uint64_t off = file_size_ - kMagicAndLengthSize - footer_size;
-  RETURN_NOT_OK(block_->Read(off, footer_size,
-                             &footer_slice, footer_space));
+  RETURN_NOT_OK(block_->Read(off, &footer_slice));
   if (!footer_->ParseFromArray(footer_slice.data(), footer_size)) {
     return Status::Corruption("Invalid cfile pb footer");
   }
@@ -365,8 +362,8 @@ Status CFileReader::ReadBlock(const BlockPointer &ptr, CacheControl cache_contro
   }
   uint8_t* buf = scratch.get();
 
-  Slice block;
-  RETURN_NOT_OK(block_->Read(ptr.offset(), ptr.size(), &block, buf));
+  Slice block(buf, ptr.size());
+  RETURN_NOT_OK(block_->Read(ptr.offset(), &block));
   if (block.size() != ptr.size()) {
     return Status::IOError("Could not read full block length");
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/consensus/log_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_util.cc b/src/kudu/consensus/log_util.cc
index 17778b4..eda0c55 100644
--- a/src/kudu/consensus/log_util.cc
+++ b/src/kudu/consensus/log_util.cc
@@ -411,12 +411,12 @@ Status ReadableLogSegment::ReadHeader() {
   }
 
   uint8_t header_space[header_size];
-  Slice header_slice;
+  Slice header_slice(header_space, header_size);
   LogSegmentHeaderPB header;
 
   // Read and parse the log segment header.
   RETURN_NOT_OK_PREPEND(readable_file_->Read(kLogSegmentHeaderMagicAndHeaderLength,
-                                             header_size, &header_slice, header_space),
+                                             &header_slice),
                         "Unable to read fully");
 
   RETURN_NOT_OK_PREPEND(pb_util::ParseFromArray(&header,
@@ -438,8 +438,8 @@ Status ReadableLogSegment::ReadHeader() {
 
 Status ReadableLogSegment::ReadHeaderMagicAndHeaderLength(uint32_t *len) {
   uint8_t scratch[kLogSegmentHeaderMagicAndHeaderLength];
-  Slice slice;
-  RETURN_NOT_OK(readable_file_->Read(0, kLogSegmentHeaderMagicAndHeaderLength, &slice, scratch));
+  Slice slice(scratch, kLogSegmentHeaderMagicAndHeaderLength);
+  RETURN_NOT_OK(readable_file_->Read(0, &slice));
   RETURN_NOT_OK(ParseHeaderMagicAndHeaderLength(slice, len));
   return Status::OK();
 }
@@ -517,15 +517,14 @@ Status ReadableLogSegment::ReadFooter() {
   }
 
   uint8_t footer_space[footer_size];
-  Slice footer_slice;
+  Slice footer_slice(footer_space, footer_size);
 
   int64_t footer_offset = file_size() - kLogSegmentFooterMagicAndFooterLength - footer_size;
 
   LogSegmentFooterPB footer;
 
   // Read and parse the log segment footer.
-  RETURN_NOT_OK_PREPEND(readable_file_->Read(footer_offset, footer_size,
-                                             &footer_slice, footer_space),
+  RETURN_NOT_OK_PREPEND(readable_file_->Read(footer_offset, &footer_slice),
                         "Footer not found. Could not read fully.");
 
   RETURN_NOT_OK_PREPEND(pb_util::ParseFromArray(&footer,
@@ -539,11 +538,11 @@ Status ReadableLogSegment::ReadFooter() {
 
 Status ReadableLogSegment::ReadFooterMagicAndFooterLength(uint32_t *len) {
   uint8_t scratch[kLogSegmentFooterMagicAndFooterLength];
-  Slice slice;
+  Slice slice(scratch, kLogSegmentFooterMagicAndFooterLength);
 
   CHECK_GT(file_size(), kLogSegmentFooterMagicAndFooterLength);
   RETURN_NOT_OK(readable_file_->Read(file_size() - kLogSegmentFooterMagicAndFooterLength,
-                                     kLogSegmentFooterMagicAndFooterLength, &slice, scratch));
+                                     &slice));
 
   RETURN_NOT_OK(ParseFooterMagicAndFooterLength(slice, len));
   return Status::OK();
@@ -600,8 +599,8 @@ Status ReadableLogSegment::ScanForValidEntryHeaders(int64_t offset, bool* has_va
        offset < file_size() - entry_header_size();
        offset += kChunkSize - entry_header_size()) {
     int rem = std::min<int64_t>(file_size() - offset, kChunkSize);
-    Slice chunk;
-    RETURN_NOT_OK(readable_file()->Read(offset, rem, &chunk, &buf[0]));
+    Slice chunk(buf.get(), rem);
+    RETURN_NOT_OK(readable_file()->Read(offset, &chunk));
 
     // Optimization for the case where a chunk is all zeros -- this is common in the
     // case of pre-allocated files. This avoids a lot of redundant CRC calculation.
@@ -641,8 +640,8 @@ Status ReadableLogSegment::ReadEntryHeaderAndBatch(int64_t* offset, faststring*
 Status ReadableLogSegment::ReadEntryHeader(int64_t *offset, EntryHeader* header) {
   const size_t header_size = entry_header_size();
   uint8_t scratch[header_size];
-  Slice slice;
-  RETURN_NOT_OK_PREPEND(readable_file()->Read(*offset, header_size, &slice, scratch),
+  Slice slice(scratch, header_size);
+  RETURN_NOT_OK_PREPEND(readable_file()->Read(*offset, &slice),
                         "Could not read log entry header");
 
   if (PREDICT_FALSE(!DecodeEntryHeader(slice, header))) {
@@ -702,12 +701,8 @@ Status ReadableLogSegment::ReadEntryBatch(int64_t *offset,
     buf_len += header.msg_length;
   }
   tmp_buf->resize(buf_len);
-  Slice entry_batch_slice;
-
-  Status s =  readable_file()->Read(*offset,
-                                    header.msg_length_compressed,
-                                    &entry_batch_slice,
-                                    tmp_buf->data());
+  Slice entry_batch_slice(tmp_buf->data(), header.msg_length_compressed);
+  Status s =  readable_file()->Read(*offset, &entry_batch_slice);
 
   if (!s.ok()) return Status::IOError(Substitute("Could not read entry. Cause: $0",
                                                  s.ToString()));

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/block_manager-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-stress-test.cc b/src/kudu/fs/block_manager-stress-test.cc
index 5b86f42..54a63d1 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -333,9 +333,9 @@ void BlockManagerStressTest<T>::ReaderThread() {
     // Read it fully into memory.
     uint64_t block_size;
     CHECK_OK(block->Size(&block_size));
-    Slice data;
     gscoped_ptr<uint8_t[]> scratch(new uint8_t[block_size]);
-    CHECK_OK(block->Read(0, block_size, &data, scratch.get()));
+    Slice data(scratch.get(), block_size);
+    CHECK_OK(block->Read(0, &data));
 
     // The first 4 bytes correspond to the PRNG seed.
     CHECK(data.size() >= 4);

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/block_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 71f5471..dca19d2 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -255,9 +255,9 @@ TYPED_TEST(BlockManagerTest, EndToEndTest) {
   uint64_t sz;
   ASSERT_OK(read_block->Size(&sz));
   ASSERT_EQ(test_data.length(), sz);
-  Slice data;
   gscoped_ptr<uint8_t[]> scratch(new uint8_t[test_data.length()]);
-  ASSERT_OK(read_block->Read(0, test_data.length(), &data, scratch.get()));
+  Slice data(scratch.get(), test_data.length());
+  ASSERT_OK(read_block->Read(0, &data));
   ASSERT_EQ(test_data, data);
 
   // We don't actually do anything with the result of this call; we just want
@@ -288,9 +288,9 @@ TYPED_TEST(BlockManagerTest, ReadAfterDeleteTest) {
               .IsNotFound());
 
   // But we should still be able to read from the opened block.
-  Slice data;
   gscoped_ptr<uint8_t[]> scratch(new uint8_t[test_data.length()]);
-  ASSERT_OK(read_block->Read(0, test_data.length(), &data, scratch.get()));
+  Slice data(scratch.get(), test_data.length());
+  ASSERT_OK(read_block->Read(0, &data));
   ASSERT_EQ(test_data, data);
 }
 
@@ -479,9 +479,9 @@ TYPED_TEST(BlockManagerTest, PersistenceTest) {
   ASSERT_OK(new_bm->OpenBlock(written_block2->id(), &read_block));
   ASSERT_OK(read_block->Size(&sz));
   ASSERT_EQ(test_data.length(), sz);
-  Slice data;
   gscoped_ptr<uint8_t[]> scratch(new uint8_t[test_data.length()]);
-  ASSERT_OK(read_block->Read(0, test_data.length(), &data, scratch.get()));
+  Slice data(scratch.get(), test_data.length());
+  ASSERT_OK(read_block->Read(0, &data));
   ASSERT_EQ(test_data, data);
   ASSERT_OK(read_block->Close());
   ASSERT_TRUE(new_bm->OpenBlock(written_block3->id(), nullptr)
@@ -562,9 +562,9 @@ TYPED_TEST(BlockManagerTest, MetricsTest) {
         i * kTestData.length(), (i + 1) * kTestData.length()));
 
     // The read is reflected in total_bytes_read.
-    Slice data;
     gscoped_ptr<uint8_t[]> scratch(new uint8_t[kTestData.length()]);
-    ASSERT_OK(reader->Read(0, kTestData.length(), &data, scratch.get()));
+    Slice data(scratch.get(), kTestData.length());
+    ASSERT_OK(reader->Read(0, &data));
     ASSERT_NO_FATAL_FAILURE(CheckMetrics(
         entity, 1, 0, i + 1, i + 1,
         (i + 1) * kTestData.length(), (i + 1) * kTestData.length()));
@@ -679,8 +679,8 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailedWrites) {
 
     for (int i = 0; i < kNumAppends; i++) {
       uint8_t buf[kTestData.size()];
-      Slice s;
-      RETURN_NOT_OK(block->Read(i * kNumAppends, sizeof(buf), &s, buf));
+      Slice s(buf, kTestData.size());
+      RETURN_NOT_OK(block->Read(i * kNumAppends, &s));
       CHECK_EQ(kTestData, s);
     }
     return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/block_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager.h b/src/kudu/fs/block_manager.h
index 8b2589d..2c92a5f 100644
--- a/src/kudu/fs/block_manager.h
+++ b/src/kudu/fs/block_manager.h
@@ -143,15 +143,11 @@ class ReadableBlock : public Block {
   // Returns the on-disk size of a written block.
   virtual Status Size(uint64_t* sz) const = 0;
 
-  // Reads exactly 'length' bytes beginning from 'offset' in the block,
-  // returning an error if fewer bytes exist. A slice referencing the
-  // results is written to 'result' and may be backed by memory in
-  // 'scratch'. As such, 'scratch' must be at least 'length' in size and
-  // must remain alive while 'result' is used.
-  //
-  // Does not modify 'result' on error (but may modify 'scratch').
-  virtual Status Read(uint64_t offset, size_t length,
-                      Slice* result, uint8_t* scratch) const = 0;
+  // Reads exactly 'result.size' bytes beginning from 'offset' in the block,
+  // returning an error if fewer bytes exist. Sets "result" to the data that
+  // was read.
+  // If an error was encountered, returns a non-OK status.
+  virtual Status Read(uint64_t offset, Slice* result) const = 0;
 
   // Returns the memory usage of this object including the object itself.
   virtual size_t memory_footprint() const = 0;

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/file_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/file_block_manager.cc b/src/kudu/fs/file_block_manager.cc
index 713e7b1..79e84d7 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -382,8 +382,7 @@ class FileReadableBlock : public ReadableBlock {
 
   virtual Status Size(uint64_t* sz) const OVERRIDE;
 
-  virtual Status Read(uint64_t offset, size_t length,
-                      Slice* result, uint8_t* scratch) const OVERRIDE;
+  virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE;
 
   virtual size_t memory_footprint() const OVERRIDE;
 
@@ -443,13 +442,12 @@ Status FileReadableBlock::Size(uint64_t* sz) const {
   return reader_->Size(sz);
 }
 
-Status FileReadableBlock::Read(uint64_t offset, size_t length,
-                               Slice* result, uint8_t* scratch) const {
+Status FileReadableBlock::Read(uint64_t offset, Slice* result) const {
   DCHECK(!closed_.Load());
 
-  RETURN_NOT_OK(reader_->Read(offset, length, result, scratch));
+  RETURN_NOT_OK(reader_->Read(offset, result));
   if (block_manager_->metrics_) {
-    block_manager_->metrics_->total_bytes_read->IncrementBy(length);
+    block_manager_->metrics_->total_bytes_read->IncrementBy(result->size());
   }
 
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/fs-test-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs-test-util.h b/src/kudu/fs/fs-test-util.h
index b99f7ad..cdfa07e 100644
--- a/src/kudu/fs/fs-test-util.h
+++ b/src/kudu/fs/fs-test-util.h
@@ -61,10 +61,9 @@ class CountingReadableBlock : public ReadableBlock {
     return block_->Size(sz);
   }
 
-  virtual Status Read(uint64_t offset, size_t length,
-                      Slice* result, uint8_t* scratch) const OVERRIDE {
-    RETURN_NOT_OK(block_->Read(offset, length, result, scratch));
-    *bytes_read_ += length;
+  virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE {
+    RETURN_NOT_OK(block_->Read(offset, result));
+    *bytes_read_ += result->size();
     return Status::OK();
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/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 1338547..6b38980 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -81,10 +81,10 @@ class FsManagerTestBase : public KuduTest {
     ASSERT_OK(writer->Close());
 
     // Test Read
-    Slice result;
+    Slice result(buffer, data.size());
     unique_ptr<fs::ReadableBlock> reader;
     ASSERT_OK(fs_manager()->OpenBlock(writer->id(), &reader));
-    ASSERT_OK(reader->Read(0, data.size(), &result, buffer));
+    ASSERT_OK(reader->Read(0, &result));
     ASSERT_EQ(data.size(), result.size());
     ASSERT_EQ(0, result.compare(data));
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/log_block_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index 1a2d828..a193ff6 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -508,9 +508,9 @@ TEST_F(LogBlockManagerTest, TestMetadataTruncation) {
   uint64_t latest_meta_size;
   ASSERT_OK(env_->GetFileSize(metadata_path, &latest_meta_size));
   ASSERT_OK(env_->NewRandomAccessFile(metadata_path, &meta_file));
-  Slice result;
   gscoped_ptr<uint8_t[]> scratch(new uint8_t[latest_meta_size]);
-  ASSERT_OK(meta_file->Read(0, latest_meta_size, &result, scratch.get()));
+  Slice result(scratch.get(), latest_meta_size);
+  ASSERT_OK(meta_file->Read(0, &result));
   string data = result.ToString();
   // Flip the high bit of the length field, which is a 4-byte little endian
   // unsigned integer. This will cause the length field to represent a large
@@ -846,8 +846,8 @@ TEST_F(LogBlockManagerTest, TestMisalignedBlocksFuzz) {
     ASSERT_OK(b->Size(&size));
     ASSERT_EQ(0, size % sizeof(raw_block_id));
     uint8_t buf[size];
-    Slice s;
-    ASSERT_OK(b->Read(0, size, &s, buf));
+    Slice s(buf, size);
+    ASSERT_OK(b->Read(0, &s));
     for (int i = 0; i < size; i += sizeof(raw_block_id)) {
       ASSERT_EQ(raw_block_id, *reinterpret_cast<uint64_t*>(buf + i));
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/fs/log_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index 7e18c33..22ef9ee 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -287,8 +287,7 @@ class LogBlockContainer {
   Status WriteData(int64_t offset, const Slice& data);
 
   // See RWFile::Read().
-  Status ReadData(int64_t offset, size_t length,
-                  Slice* result, uint8_t* scratch) const;
+  Status ReadData(int64_t offset, Slice* result) const;
 
   // Appends 'pb' to this container's metadata file.
   //
@@ -831,11 +830,10 @@ Status LogBlockContainer::WriteData(int64_t offset, const Slice& data) {
   return Status::OK();
 }
 
-Status LogBlockContainer::ReadData(int64_t offset, size_t length,
-                                   Slice* result, uint8_t* scratch) const {
+Status LogBlockContainer::ReadData(int64_t offset, Slice* result) const {
   DCHECK_GE(offset, 0);
 
-  return data_file_->Read(offset, length, result, scratch);
+  return data_file_->Read(offset, result);
 }
 
 Status LogBlockContainer::AppendMetadata(const BlockRecordPB& pb) {
@@ -1249,8 +1247,7 @@ class LogReadableBlock : public ReadableBlock {
 
   virtual Status Size(uint64_t* sz) const OVERRIDE;
 
-  virtual Status Read(uint64_t offset, size_t length,
-                      Slice* result, uint8_t* scratch) const OVERRIDE;
+  virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE;
 
   virtual size_t memory_footprint() const OVERRIDE;
 
@@ -1306,22 +1303,21 @@ Status LogReadableBlock::Size(uint64_t* sz) const {
   return Status::OK();
 }
 
-Status LogReadableBlock::Read(uint64_t offset, size_t length,
-                              Slice* result, uint8_t* scratch) const {
+Status LogReadableBlock::Read(uint64_t offset, Slice* result) const {
   DCHECK(!closed_.Load());
 
   uint64_t read_offset = log_block_->offset() + offset;
-  if (log_block_->length() < offset + length) {
+  if (log_block_->length() < offset + result->size()) {
     return Status::IOError("Out-of-bounds read",
                            Substitute("read of [$0-$1) in block [$2-$3)",
                                       read_offset,
-                                      read_offset + length,
+                                      read_offset + result->size(),
                                       log_block_->offset(),
                                       log_block_->offset() + log_block_->length()));
   }
 
   MicrosecondsInt64 start_time = GetMonoTimeMicros();
-  RETURN_NOT_OK(container_->ReadData(read_offset, length, result, scratch));
+  RETURN_NOT_OK(container_->ReadData(read_offset, result));
   MicrosecondsInt64 end_time = GetMonoTimeMicros();
 
   int64_t dur = end_time - start_time;
@@ -1331,7 +1327,7 @@ Status LogReadableBlock::Read(uint64_t offset, size_t length,
   TRACE_COUNTER_INCREMENT(counter, 1);
 
   if (container_->metrics()) {
-    container_->metrics()->generic_metrics.total_bytes_read->IncrementBy(length);
+    container_->metrics()->generic_metrics.total_bytes_read->IncrementBy(result->size());
   }
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/tserver/tablet_copy-test-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy-test-base.h b/src/kudu/tserver/tablet_copy-test-base.h
index 92de7ba..2d5fc9a 100644
--- a/src/kudu/tserver/tablet_copy-test-base.h
+++ b/src/kudu/tserver/tablet_copy-test-base.h
@@ -107,13 +107,8 @@ class TabletCopyTest : public TabletServerTestBase {
     uint64_t size = 0;
     RETURN_NOT_OK(block->Size(&size));
     scratch->resize(size);
-    RETURN_NOT_OK(block->Read(0, size, slice, scratch->data()));
-
-    // Since the mmap will go away on return, copy the data into scratch.
-    if (slice->data() != scratch->data()) {
-      memcpy(scratch->data(), slice->data(), slice->size());
-      *slice = Slice(scratch->data(), slice->size());
-    }
+    *slice = Slice(scratch->data(), size);
+    RETURN_NOT_OK(block->Read(0, slice));
     return Status::OK();
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/tserver/tablet_copy_client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_client-test.cc b/src/kudu/tserver/tablet_copy_client-test.cc
index 040acc3..b775c58 100644
--- a/src/kudu/tserver/tablet_copy_client-test.cc
+++ b/src/kudu/tserver/tablet_copy_client-test.cc
@@ -77,12 +77,13 @@ Status TabletCopyClientTest::CompareFileContents(const string& path1, const stri
                               strings::Substitute("$0 vs $1 bytes", size1, size2));
   }
 
-  Slice slice1, slice2;
   faststring scratch1, scratch2;
   scratch1.resize(size1);
   scratch2.resize(size2);
-  RETURN_NOT_OK(file1->Read(0, size1, &slice1, scratch1.data()));
-  RETURN_NOT_OK(file2->Read(0, size2, &slice2, scratch2.data()));
+  Slice slice1(scratch1.data(), size1);
+  Slice slice2(scratch2.data(), size2);
+  RETURN_NOT_OK(file1->Read(0, &slice1));
+  RETURN_NOT_OK(file2->Read(0, &slice2));
   int result = strings::fastmemcmp_inlined(slice1.data(), slice2.data(), size1);
   if (result != 0) {
     return Status::Corruption("Files do not match");

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/tserver/tablet_copy_service-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_service-test.cc b/src/kudu/tserver/tablet_copy_service-test.cc
index c71e49e..72346f9 100644
--- a/src/kudu/tserver/tablet_copy_service-test.cc
+++ b/src/kudu/tserver/tablet_copy_service-test.cc
@@ -455,8 +455,8 @@ TEST_F(TabletCopyServiceTest, TestFetchLog) {
   faststring scratch;
   int64_t size = segment->file_size();
   scratch.resize(size);
-  Slice slice;
-  ASSERT_OK(segment->readable_file()->Read(0, size, &slice, scratch.data()));
+  Slice slice(scratch.data(), size);
+  ASSERT_OK(segment->readable_file()->Read(0, &slice));
 
   AssertDataEqual(slice.data(), slice.size(), resp.chunk());
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/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 2d9ec1d..80d270a 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -266,8 +266,8 @@ TEST_F(TabletCopyTest, TestBlocksEqual) {
       ASSERT_OK(Env::Default()->GetFileSize(path, &session_block_size));
       faststring buf;
       buf.resize(session_block_size);
-      Slice data;
-      ASSERT_OK(file->Read(session_block_size, &data, buf.data()));
+      Slice data(buf.data(), session_block_size);
+      ASSERT_OK(file->Read(&data));
       uint32_t session_crc = crc::Crc32c(data.data(), data.size());
       LOG(INFO) << "session block file has size of " << session_block_size
                 << " and CRC32C of " << session_crc << ": " << path;
@@ -277,7 +277,8 @@ TEST_F(TabletCopyTest, TestBlocksEqual) {
       uint64_t tablet_block_size = 0;
       ASSERT_OK(tablet_block->Size(&tablet_block_size));
       buf.resize(tablet_block_size);
-      ASSERT_OK(tablet_block->Read(0, tablet_block_size, &data, buf.data()));
+      Slice data2(buf.data(), tablet_block_size);
+      ASSERT_OK(tablet_block->Read(0, &data2));
       uint32_t tablet_crc = crc::Crc32c(data.data(), data.size());
       LOG(INFO) << "tablet block file has size of " << tablet_block_size
                 << " and CRC32C of " << tablet_crc

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/tserver/tablet_copy_source_session.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_source_session.cc b/src/kudu/tserver/tablet_copy_source_session.cc
index fe9917d..17b73f7 100644
--- a/src/kudu/tserver/tablet_copy_source_session.cc
+++ b/src/kudu/tserver/tablet_copy_source_session.cc
@@ -217,8 +217,8 @@ static Status ReadFileChunkToBuf(const Info* info,
   // Violates the API contract, but avoids excessive copies.
   data->resize(response_data_size);
   uint8_t* buf = reinterpret_cast<uint8_t*>(const_cast<char*>(data->data()));
-  Slice slice;
-  Status s = info->Read(offset, response_data_size, &slice, buf);
+  Slice slice(buf, response_data_size);
+  Status s = info->Read(offset, &slice);
   if (PREDICT_FALSE(!s.ok())) {
     s = s.CloneAndPrepend(
         Substitute("Unable to read existing file for $0", data_name));

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/tserver/tablet_copy_source_session.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy_source_session.h b/src/kudu/tserver/tablet_copy_source_session.h
index 20f5a2e..0d8aad1 100644
--- a/src/kudu/tserver/tablet_copy_source_session.h
+++ b/src/kudu/tserver/tablet_copy_source_session.h
@@ -58,8 +58,8 @@ struct ImmutableRandomAccessFileInfo {
                                 int64_t size)
       : readable(std::move(readable)), size(size) {}
 
-  Status Read(uint64_t offset, int64_t size, Slice* data, uint8_t* scratch) const {
-    return readable->Read(offset, size, data, scratch);
+  Status Read(uint64_t offset, Slice* data) const {
+    return readable->Read(offset, data);
   }
 };
 
@@ -75,8 +75,8 @@ struct ImmutableReadableBlockInfo {
     size(size) {
   }
 
-  Status Read(uint64_t offset, int64_t size, Slice* data, uint8_t* scratch) const {
-    return readable->Read(offset, size, data, scratch);
+  Status Read(uint64_t offset, Slice* data) const {
+    return readable->Read(offset, data);
   }
 };
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 6e40891..a135f4a 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -136,8 +136,8 @@ class TestEnv : public KuduTest {
 
   void ReadAndVerifyTestData(RandomAccessFile* raf, size_t offset, size_t n) {
     unique_ptr<uint8_t[]> scratch(new uint8_t[n]);
-    Slice s;
-    ASSERT_OK(raf->Read(offset, n, &s, scratch.get()));
+    Slice s(scratch.get(), n);
+    ASSERT_OK(raf->Read(offset, &s));
     ASSERT_EQ(n, s.size());
     ASSERT_NO_FATAL_FAILURE(VerifyTestData(s, offset));
   }
@@ -367,9 +367,9 @@ TEST_F(TestEnv, TestTruncate) {
   // Read the whole file. Ensure it is all zeroes.
   unique_ptr<RandomAccessFile> raf;
   ASSERT_OK(env_->NewRandomAccessFile(test_path, &raf));
-  Slice s;
   unique_ptr<uint8_t[]> scratch(new uint8_t[size]);
-  ASSERT_OK(raf->Read(0, size, &s, scratch.get()));
+  Slice s(scratch.get(), size);
+  ASSERT_OK(raf->Read(0, &s));
   const uint8_t* data = s.data();
   for (int i = 0; i < size; i++) {
     ASSERT_EQ(0, data[i]) << "Not null at position " << i;
@@ -403,14 +403,14 @@ TEST_F(TestEnv, TestReadFully) {
   ASSERT_OK(env_util::OpenFileForRandom(env, kTestPath, &raf));
 
   const int kReadLength = 10000;
-  Slice s;
   unique_ptr<uint8_t[]> scratch(new uint8_t[kReadLength]);
+  Slice s(scratch.get(), kReadLength);
 
   // Force a short read to half the data length
   FLAGS_env_inject_short_read_bytes = kReadLength / 2;
 
   // Verify that Read fully reads the whole requested data.
-  ASSERT_OK(raf->Read(0, kReadLength, &s, scratch.get()));
+  ASSERT_OK(raf->Read(0, &s));
   ASSERT_EQ(s.data(), scratch.get()) << "Should have returned a contiguous copy";
   ASSERT_EQ(kReadLength, s.size());
 
@@ -421,7 +421,8 @@ TEST_F(TestEnv, TestReadFully) {
   FLAGS_env_inject_short_read_bytes = 0;
 
   // Verify that Read fails with an IOError at EOF.
-  Status status = raf->Read(kFileSize - 100, 200, &s, scratch.get());
+  Slice s2(scratch.get(), 200);
+  Status status = raf->Read(kFileSize - 100, &s2);
   ASSERT_FALSE(status.ok());
   ASSERT_TRUE(status.IsIOError());
   ASSERT_STR_CONTAINS(status.ToString(), "EOF");
@@ -507,9 +508,9 @@ TEST_F(TestEnv, TestReopen) {
   uint64_t size;
   ASSERT_OK(reader->Size(&size));
   ASSERT_EQ(first.length() + second.length(), size);
-  Slice s;
   uint8_t scratch[size];
-  ASSERT_OK(reader->Read(0, size, &s, scratch));
+  Slice s(scratch, size);
+  ASSERT_OK(reader->Read(0, &s));
   ASSERT_EQ(first + second, s.ToString());
 }
 
@@ -693,9 +694,9 @@ TEST_F(TestEnv, TestRWFile) {
   ASSERT_OK(file->Write(0, kTestData));
 
   // Read from it.
-  Slice result;
   unique_ptr<uint8_t[]> scratch(new uint8_t[kTestData.length()]);
-  ASSERT_OK(file->Read(0, kTestData.length(), &result, scratch.get()));
+  Slice result(scratch.get(), kTestData.length());
+  ASSERT_OK(file->Read(0, &result));
   ASSERT_EQ(result, kTestData);
   uint64_t sz;
   ASSERT_OK(file->Size(&sz));
@@ -707,10 +708,11 @@ TEST_F(TestEnv, TestRWFile) {
   ASSERT_OK(file->Write(1, kTestData));
   string kNewTestData = "aabcdebcdeabcde";
   unique_ptr<uint8_t[]> scratch2(new uint8_t[kNewTestData.length()]);
-  ASSERT_OK(file->Read(0, kNewTestData.length(), &result, scratch2.get()));
+  Slice result2(scratch2.get(), kNewTestData.length());
+  ASSERT_OK(file->Read(0, &result2));
 
   // Retest.
-  ASSERT_EQ(result, kNewTestData);
+  ASSERT_EQ(result2, kNewTestData);
   ASSERT_OK(file->Size(&sz));
   ASSERT_EQ(kNewTestData.length(), sz);
 
@@ -722,8 +724,10 @@ TEST_F(TestEnv, TestRWFile) {
   // Reopen it without truncating the existing data.
   opts.mode = Env::OPEN_EXISTING;
   ASSERT_OK(env_->NewRWFile(opts, GetTestPath("foo"), &file));
-  ASSERT_OK(file->Read(0, kNewTestData.length(), &result, scratch2.get()));
-  ASSERT_EQ(result, kNewTestData);
+  unique_ptr<uint8_t[]> scratch3(new uint8_t[kNewTestData.length()]);
+  Slice result3(scratch3.get(), kNewTestData.length());
+  ASSERT_OK(file->Read(0, &result3));
+  ASSERT_EQ(result3, kNewTestData);
 }
 
 TEST_F(TestEnv, TestCanonicalize) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/env.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.cc b/src/kudu/util/env.cc
index 9f8e7ba..1f6478c 100644
--- a/src/kudu/util/env.cc
+++ b/src/kudu/util/env.cc
@@ -74,8 +74,8 @@ Status ReadFileToString(Env* env, const std::string& fname, faststring* data) {
   static const int kBufferSize = 8192;
   unique_ptr<uint8_t[]> scratch(new uint8_t[kBufferSize]);
   while (true) {
-    Slice fragment;
-    s = file->Read(kBufferSize, &fragment, scratch.get());
+    Slice fragment(scratch.get(), kBufferSize);
+    s = file->Read(&fragment);
     if (!s.ok()) {
       break;
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/env.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 1ed393d..d7b68aa 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -346,15 +346,13 @@ class SequentialFile {
   SequentialFile() { }
   virtual ~SequentialFile();
 
-  // Read up to "n" bytes from the file.  "scratch[0..n-1]" may be
-  // written by this routine.  Sets "*result" to the data that was
-  // read (including if fewer than "n" bytes were successfully read).
-  // May set "*result" to point at data in "scratch[0..n-1]", so
-  // "scratch[0..n-1]" must be live when "*result" is used.
-  // If an error was encountered, returns a non-OK status.
+  // Read up to "result.size" bytes from the file.
+  // Sets "result.data" to the data that was read.
+  // If an error was encountered, returns a non-OK status
+  // and the contents of "result" are invalid.
   //
   // REQUIRES: External synchronization
-  virtual Status Read(size_t n, Slice* result, uint8_t *scratch) = 0;
+  virtual Status Read(Slice* result) = 0;
 
   // Skip "n" bytes from the file. This is guaranteed to be no
   // slower that reading the same data, but may be faster.
@@ -375,17 +373,16 @@ class RandomAccessFile {
   RandomAccessFile() { }
   virtual ~RandomAccessFile();
 
-  // Read up to "n" bytes from the file starting at "offset".
-  // "scratch[0..n-1]" may be written by this routine.  Sets "*result"
-  // to the data that was read (including if fewer than "n" bytes were
-  // successfully read).  May set "*result" to point at data in
-  // "scratch[0..n-1]", so "scratch[0..n-1]" must be live when
-  // "*result" is used.  If an error was encountered, returns a non-OK
-  // status.
+  // Read up to "result.size" from the file starting at "offset".
+  // Sets "result.data" to the data that was read.
+  // If an error was encountered, returns a non-OK status.
+  //
+  // 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 n, Slice* result,
-                      uint8_t *scratch) const = 0;
+  virtual Status Read(uint64_t offset, Slice* result) const = 0;
 
   // Returns the size of the file
   virtual Status Size(uint64_t *size) const = 0;
@@ -504,10 +501,8 @@ class RWFile {
 
   virtual ~RWFile();
 
-  // Read exactly 'length' bytes from the file starting at 'offset'.
-  // 'scratch[0..length-1]' may be written by this routine. Sets '*result'
-  // to the data that was read. May set '*result' to point at data in
-  // 'scratch[0..length-1]', which must be live when '*result' is used.
+  // Read up to "result.size" from the file starting at "offset".
+  // Sets "result.data" to the data that was read.
   // If an error was encountered, returns a non-OK status.
   //
   // This method will internally retry on EINTR and "short reads" in order to
@@ -515,8 +510,7 @@ class RWFile {
   // 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,
-                      Slice* result, uint8_t* scratch) const = 0;
+  virtual Status Read(uint64_t offset, Slice* result) const = 0;
 
   // Writes 'data' to the file position given by 'offset'.
   virtual Status Write(uint64_t offset, const Slice& data) = 0;

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/env_posix.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index b9ed40f..8932c83 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -255,20 +255,20 @@ Status DoOpen(const string& filename, Env::CreateMode mode, int* fd) {
   return Status::OK();
 }
 
-Status DoRead(int fd, const string& filename, uint64_t offset, size_t length,
-                     Slice* result, uint8_t *scratch) {
+Status DoRead(int fd, const string& filename, uint64_t offset, Slice* result) {
   ThreadRestrictions::AssertIOAllowed();
-  size_t rem = length;
-  uint8_t* dst = scratch;
+  uint64_t cur_offset = offset;
+  uint8_t* dst = result->mutable_data();
+  size_t rem = result->size();
   while (rem > 0) {
     size_t req = rem;
     // Inject a short read for testing
-    if (PREDICT_FALSE(FLAGS_env_inject_short_read_bytes > 0 && req == length)) {
+    if (PREDICT_FALSE(FLAGS_env_inject_short_read_bytes > 0 && req == result->size())) {
       DCHECK_LT(FLAGS_env_inject_short_read_bytes, req);
       req -= FLAGS_env_inject_short_read_bytes;
     }
     ssize_t r;
-    RETRY_ON_EINTR(r, pread(fd, dst, req, offset));
+    RETRY_ON_EINTR(r, pread(fd, dst, req, cur_offset));
     if (r < 0) {
       // An error: return a non-ok status.
       return IOError(filename, errno);
@@ -276,15 +276,14 @@ Status DoRead(int fd, const string& filename, uint64_t offset, size_t length,
     if (r == 0) {
       // EOF
       return Status::IOError(Substitute("EOF trying to read $0 bytes at offset $1",
-                                        length, offset));
+                                        result->size(), offset));
     }
     DCHECK_LE(r, rem);
     dst += r;
     rem -= r;
-    offset += r;
+    cur_offset += r;
   }
   DCHECK_EQ(0, rem);
-  *result = Slice(scratch, length);
   return Status::OK();
 }
 
@@ -298,21 +297,22 @@ class PosixSequentialFile: public SequentialFile {
       : filename_(std::move(fname)), file_(f) {}
   virtual ~PosixSequentialFile() { fclose(file_); }
 
-  virtual Status Read(size_t n, Slice* result, uint8_t* scratch) OVERRIDE {
+  virtual Status Read(Slice* result) OVERRIDE {
     ThreadRestrictions::AssertIOAllowed();
-    Status s;
     size_t r;
-    STREAM_RETRY_ON_EINTR(r, file_, fread_unlocked(scratch, 1, n, file_));
-    *result = Slice(scratch, r);
-    if (r < n) {
+    STREAM_RETRY_ON_EINTR(r, file_, fread_unlocked(result->mutable_data(), 1,
+                                                   result->size(), file_));
+    if (r < result->size()) {
       if (feof(file_)) {
-        // We leave status as ok if we hit the end of the file
+        // We leave status as ok if we hit the end of the file.
+        // We need to adjust the slice size.
+        result->truncate(r);
       } else {
         // A partial read with an error: return a non-ok status.
-        s = IOError(filename_, errno);
+        return IOError(filename_, errno);
       }
     }
-    return s;
+    return Status::OK();
   }
 
   virtual Status Skip(uint64_t n) OVERRIDE {
@@ -338,9 +338,8 @@ class PosixRandomAccessFile: public RandomAccessFile {
       : filename_(std::move(fname)), fd_(fd) {}
   virtual ~PosixRandomAccessFile() { close(fd_); }
 
-  virtual Status Read(uint64_t offset, size_t n, Slice* result,
-                      uint8_t *scratch) const OVERRIDE {
-    return DoRead(fd_, filename_, offset, n, result, scratch);
+  virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE {
+    return DoRead(fd_, filename_, offset, result);
   }
 
   virtual Status Size(uint64_t *size) const OVERRIDE {
@@ -581,9 +580,8 @@ class PosixRWFile : public RWFile {
     WARN_NOT_OK(Close(), "Failed to close " + filename_);
   }
 
-  virtual Status Read(uint64_t offset, size_t length,
-                      Slice* result, uint8_t* scratch) const OVERRIDE {
-    return DoRead(fd_, filename_, offset, length, result, scratch);
+  virtual Status Read(uint64_t offset, Slice* result) const OVERRIDE {
+    return DoRead(fd_, filename_, offset, result);
   }
 
   virtual Status Write(uint64_t offset, const Slice& data) OVERRIDE {

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/env_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc
index 72e1b10..44f687a 100644
--- a/src/kudu/util/env_util.cc
+++ b/src/kudu/util/env_util.cc
@@ -199,8 +199,8 @@ Status CopyFile(Env* env, const string& source_path, const string& dest_path,
   uint64_t bytes_read = 0;
   while (bytes_read < size) {
     uint64_t max_bytes_to_read = std::min<uint64_t>(size - bytes_read, kBufferSize);
-    Slice data;
-    RETURN_NOT_OK(source->Read(max_bytes_to_read, &data, scratch.get()));
+    Slice data(scratch.get(), max_bytes_to_read);
+    RETURN_NOT_OK(source->Read(&data));
     RETURN_NOT_OK(dest->Append(data));
     bytes_read += data.size();
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/file_cache-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache-stress-test.cc b/src/kudu/util/file_cache-stress-test.cc
index 1abec57..966f202 100644
--- a/src/kudu/util/file_cache-stress-test.cc
+++ b/src/kudu/util/file_cache-stress-test.cc
@@ -247,8 +247,8 @@ class FileCacheStressTest : public KuduTest {
     uint64_t off = file_size > 0 ? rand->Uniform(file_size) : 0;
     size_t len = file_size > 0 ? rand->Uniform(file_size - off) : 0;
     unique_ptr<uint8_t[]> scratch(new uint8_t[len]);
-    Slice s;
-    RETURN_NOT_OK(file->Read(off, len, &s, scratch.get()));
+    Slice s(scratch.get(), len);
+    RETURN_NOT_OK(file->Read(off, &s));
 
     (*metrics)[BaseName(file->filename())]["read"]++;
     return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/file_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache-test.cc b/src/kudu/util/file_cache-test.cc
index 28a0e3e..5ac568e 100644
--- a/src/kudu/util/file_cache-test.cc
+++ b/src/kudu/util/file_cache-test.cc
@@ -253,8 +253,8 @@ TYPED_TEST(FileCacheTest, TestHeavyReads) {
     const auto& f = opened_files[idx];
     uint64_t size;
     ASSERT_OK(f->Size(&size));
-    Slice s;
-    ASSERT_OK(f->Read(0, size, &s, buf.get()));
+    Slice s(buf.get(), size);
+    ASSERT_OK(f->Read(0, &s));
     ASSERT_EQ(data, s);
     ASSERT_LE(CountOpenFds(this->env_),
               this->initial_open_fds_ + kCacheCapacity);

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/file_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache.cc b/src/kudu/util/file_cache.cc
index fc272af..6abdf84 100644
--- a/src/kudu/util/file_cache.cc
+++ b/src/kudu/util/file_cache.cc
@@ -206,11 +206,10 @@ class Descriptor<RWFile> : public RWFile {
 
   ~Descriptor() = default;
 
-  Status Read(uint64_t offset, size_t length,
-              Slice* result, uint8_t* scratch) const override {
+  Status Read(uint64_t offset, Slice* result) const override {
     ScopedOpenedDescriptor<RWFile> opened(&base_);
     RETURN_NOT_OK(ReopenFileIfNecessary(&opened));
-    return opened.file()->Read(offset, length, result, scratch);
+    return opened.file()->Read(offset, result);
   }
 
   Status Write(uint64_t offset, const Slice& data) override {
@@ -325,11 +324,10 @@ class Descriptor<RandomAccessFile> : public RandomAccessFile {
 
   ~Descriptor() = default;
 
-  Status Read(uint64_t offset, size_t n,
-              Slice* result, uint8_t *scratch) const override {
+  Status Read(uint64_t offset, Slice* result) const override {
     ScopedOpenedDescriptor<RandomAccessFile> opened(&base_);
     RETURN_NOT_OK(ReopenFileIfNecessary(&opened));
-    return opened.file()->Read(offset, n, result, scratch);
+    return opened.file()->Read(offset, result);
   }
 
   Status Size(uint64_t *size) const override {

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/pb_util-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util-internal.cc b/src/kudu/util/pb_util-internal.cc
index b439327..d891d75 100644
--- a/src/kudu/util/pb_util-internal.cc
+++ b/src/kudu/util/pb_util-internal.cc
@@ -39,17 +39,13 @@ bool SequentialFileFileInputStream::Next(const void **data, int *size) {
     return true;
   }
 
-  Slice result;
-  status_ = rfile_->Read(buffer_size_, &result, buffer_.get());
+  Slice result(buffer_.get(), buffer_size_);
+  status_ = rfile_->Read(&result);
   if (!status_.ok()) {
     LOG(WARNING) << "Read at " << buffer_offset_ << " failed: " << status_.ToString();
     return false;
   }
 
-  if (result.data() != buffer_.get()) {
-    memcpy(buffer_.get(), result.data(), result.size());
-  }
-
   buffer_used_ = result.size();
   buffer_offset_ = buffer_used_;
   total_read_ += buffer_used_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/pb_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util-test.cc b/src/kudu/util/pb_util-test.cc
index c1e81c7..1d150f0 100644
--- a/src/kudu/util/pb_util-test.cc
+++ b/src/kudu/util/pb_util-test.cc
@@ -146,10 +146,10 @@ Status TestPBUtil::BitFlipFileByteRange(const string& path, uint64_t offset, uin
     RETURN_NOT_OK(env_->NewRandomAccessFile(path, &file));
     uint64_t size;
     RETURN_NOT_OK(file->Size(&size));
-    Slice slice;
     faststring scratch;
     scratch.resize(size);
-    RETURN_NOT_OK(file->Read(0, size, &slice, scratch.data()));
+    Slice slice(scratch.data(), size);
+    RETURN_NOT_OK(file->Read(0, &slice));
     buf.append(slice.data(), slice.size());
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/pb_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index f8f8d5e..fe45998 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -176,24 +176,6 @@ bool IsSupportedContainerVersion(uint32_t version) {
   return false;
 }
 
-// Perform a non-short read. The contract is that we may go to great lengths to
-// retry reads, but if we are ultimately unable to read 'length' bytes from the
-// file then a non-OK Status is returned.
-template<typename T>
-Status NonShortRead(T* file, uint64_t offset, uint64_t length, Slice* result, uint8_t* scratch);
-
-template<>
-Status NonShortRead<RandomAccessFile>(RandomAccessFile* file, uint64_t offset, uint64_t length,
-                                      Slice* result, uint8_t* scratch) {
-  return file->Read(offset, length, result, scratch);
-}
-
-template<>
-Status NonShortRead<RWFile>(RWFile* file, uint64_t offset, uint64_t length,
-                            Slice* result, uint8_t* scratch) {
-  return file->Read(offset, length, result, scratch);
-}
-
 // Reads exactly 'length' bytes from the container file into 'scratch',
 // validating that there is sufficient data in the file to read this length
 // before attempting to do so, and validating that it has read that length
@@ -219,9 +201,9 @@ Status ValidateAndReadData(ReadableFileType* reader, uint64_t file_size,
   }
 
   // Perform the read.
-  Slice s;
   unique_ptr<uint8_t[]> local_scratch(new uint8_t[length]);
-  RETURN_NOT_OK(NonShortRead(reader, *offset, length, &s, local_scratch.get()));
+  Slice s(local_scratch.get(), length);
+  RETURN_NOT_OK(reader->Read(*offset, &s));
   CHECK_EQ(length, s.size()) // Should never trigger due to contract with reader APIs.
       << Substitute("Unexpected short read: Proto container file $0: Tried to read $1 bytes "
                     "but only read $2 bytes",

http://git-wip-us.apache.org/repos/asf/kudu/blob/986ade9b/src/kudu/util/rolling_log.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/rolling_log.cc b/src/kudu/util/rolling_log.cc
index 0de25dc..486abc0 100644
--- a/src/kudu/util/rolling_log.cc
+++ b/src/kudu/util/rolling_log.cc
@@ -233,8 +233,8 @@ Status RollingLog::CompressFile(const std::string& path) const {
   // Loop reading data from the input file and writing to the gzip stream.
   uint8_t buf[32 * 1024];
   while (true) {
-    Slice result;
-    RETURN_NOT_OK_PREPEND(in_file->Read(arraysize(buf), &result, buf),
+    Slice result(buf, arraysize(buf));
+    RETURN_NOT_OK_PREPEND(in_file->Read(&result),
                           "Unable to read from gzip input");
     if (result.size() == 0) {
       break;


Mime
View raw message