Skip to content

Commit

Permalink
feat: Support o_direct flag for read file (facebookincubator#12138)
Browse files Browse the repository at this point in the history
Summary: Pull Request resolved: facebookincubator#12138

Reviewed By: yuandagits

Differential Revision: D68468301
  • Loading branch information
zacw7 authored and facebook-github-bot committed Jan 22, 2025
1 parent f81a8c4 commit 0a874f3
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 15 deletions.
4 changes: 2 additions & 2 deletions velox/common/caching/SsdFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ SsdFile::SsdFile(const Config& config)
process::TraceContext trace("SsdFile::SsdFile");
filesystems::FileOptions fileOptions;
fileOptions.shouldThrowOnFileAlreadyExists = false;
fileOptions.bufferWrite = !FLAGS_ssd_odirect;
fileOptions.bufferIo = !FLAGS_ssd_odirect;
writeFile_ = fs_->openFileForWrite(fileName_, fileOptions);
readFile_ = fs_->openFileForRead(fileName_);
readFile_ = fs_->openFileForRead(fileName_, fileOptions);

const uint64_t size = writeFile_->size();
numRegions_ = std::min<int32_t>(size / kRegionSize, maxRegions_);
Expand Down
15 changes: 11 additions & 4 deletions velox/common/file/File.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,15 @@ uint64_t InMemoryWriteFile::size() const {
return file_->size();
}

LocalReadFile::LocalReadFile(std::string_view path) : path_(path) {
fd_ = open(path_.c_str(), O_RDONLY);
LocalReadFile::LocalReadFile(std::string_view path, bool bufferIo)
: path_(path) {
int32_t flags = O_RDONLY;
#ifdef linux
if (!bufferIo) {
flags |= O_DIRECT;
}
#endif // linux
fd_ = open(path_.c_str(), flags);
if (fd_ < 0) {
if (errno == ENOENT) {
VELOX_FILE_NOT_FOUND_ERROR("No such file or directory: {}", path);
Expand Down Expand Up @@ -259,7 +266,7 @@ LocalWriteFile::LocalWriteFile(
std::string_view path,
bool shouldCreateParentDirectories,
bool shouldThrowOnFileAlreadyExists,
bool bufferWrite)
bool bufferIo)
: path_(path) {
const auto dir = fs::path(path_).parent_path();
if (shouldCreateParentDirectories && !fs::exists(dir)) {
Expand All @@ -274,7 +281,7 @@ LocalWriteFile::LocalWriteFile(
flags |= O_EXCL;
}
#ifdef linux
if (!bufferWrite) {
if (!bufferIo) {
flags |= O_DIRECT;
}
#endif // linux
Expand Down
4 changes: 2 additions & 2 deletions velox/common/file/File.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class InMemoryWriteFile final : public WriteFile {
/// files match against any filepath starting with '/'.
class LocalReadFile final : public ReadFile {
public:
explicit LocalReadFile(std::string_view path);
explicit LocalReadFile(std::string_view path, bool bufferIo = true);

/// TODO: deprecate this after creating local file all through velox fs
/// interface.
Expand Down Expand Up @@ -327,7 +327,7 @@ class LocalWriteFile final : public WriteFile {
std::string_view path,
bool shouldCreateParentDirectories = false,
bool shouldThrowOnFileAlreadyExists = true,
bool bufferWrite = true);
bool bufferIo = true);

~LocalWriteFile();

Expand Down
6 changes: 3 additions & 3 deletions velox/common/file/FileSystems.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ class LocalFileSystem : public FileSystem {

std::unique_ptr<ReadFile> openFileForRead(
std::string_view path,
const FileOptions& /*unused*/) override {
return std::make_unique<LocalReadFile>(extractPath(path));
const FileOptions& options) override {
return std::make_unique<LocalReadFile>(extractPath(path), options.bufferIo);
}

std::unique_ptr<WriteFile> openFileForWrite(
Expand All @@ -108,7 +108,7 @@ class LocalFileSystem : public FileSystem {
extractPath(path),
options.shouldCreateParentDirectories,
options.shouldThrowOnFileAlreadyExists,
options.bufferWrite);
options.bufferIo);
}

void remove(std::string_view path) override {
Expand Down
6 changes: 2 additions & 4 deletions velox/common/file/FileSystems.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,10 @@ struct FileOptions {
/// NOTE: this only applies for write open file.
bool shouldThrowOnFileAlreadyExists{true};

/// Whether to buffer the write data in file system client or not. For local
/// Whether to buffer the data in file system client or not. For local
/// filesystem on Unix-like operating system, this corresponds to the direct
/// IO mode if set.
///
/// NOTE: this only applies for write open file.
bool bufferWrite{true};
bool bufferIo{true};
};

/// Defines directory options
Expand Down

0 comments on commit 0a874f3

Please sign in to comment.