Disable readahead when using mmap for reads

Summary:
`ReadaheadRandomAccessFile` had an unwritten assumption, which was that its wrapped file's `Read()` function always copies into the provided scratch buffer. Actually this was not true when the wrapped file was `PosixMmapReadableFile`, whose `Read()` implementation does no copying and instead returns a `Slice` pointing directly into the  `mmap`'d memory region. This PR:

- prevents `ReadaheadRandomAccessFile` from ever wrapping mmap readable files
- adds an assert for the assumption `ReadaheadRandomAccessFile` makes about the wrapped file's use of scratch buffer
Closes https://github.com/facebook/rocksdb/pull/3813

Differential Revision: D7891513

Pulled By: ajkr

fbshipit-source-id: dc64a55222d6af280c39a1852ee39e9e9d7cde7d
main
Andrew Kryczka 7 years ago committed by Facebook Github Bot
parent 1d9f24dc9a
commit 4bf169f07e
  1. 6
      db/table_cache.cc
  2. 1
      util/file_reader_writer.cc

@ -98,7 +98,11 @@ Status TableCache::GetTableReader(
RecordTick(ioptions_.statistics, NO_FILE_OPENS);
if (s.ok()) {
if (readahead > 0) {
if (readahead > 0 && !env_options.use_mmap_reads) {
// Not compatible with mmap files since ReadaheadRandomAccessFile requires
// its wrapped file's Read() to copy data into the provided scratch
// buffer, which mmap files don't use.
// TODO(ajkr): try madvise for mmap files in place of buffered readahead.
file = NewReadaheadRandomAccessFile(std::move(file), readahead);
}
if (!sequential_mode && ioptions_.advise_random_on_open) {

@ -622,6 +622,7 @@ class ReadaheadRandomAccessFile : public RandomAccessFile {
if (s.ok()) {
buffer_offset_ = offset;
buffer_len_ = result.size();
assert(buffer_.BufferStart() == result.data());
}
return s;
}

Loading…
Cancel
Save