Skip to content

Commit

Permalink
test: Disable O_DIRECT in cache fuzzer (#12154)
Browse files Browse the repository at this point in the history
Summary:

O_DIRECT requires I/O size needs to be the same as a disk file block size
which is not handled in SSD cache. Misalignment can lead to EINVAL in some
filesystem and kernel version.

Reviewed By: xiaoxmeng, kagamiori, yuandagits

Differential Revision: D68562695
  • Loading branch information
zacw7 authored and facebook-github-bot committed Jan 27, 2025
1 parent cc264cd commit 9a3af71
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
4 changes: 2 additions & 2 deletions velox/common/file/tests/FaultyFileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class FaultyFileSystem : public FileSystem {
// Extracts the delegated real file path by removing the faulty file system
// scheme prefix.
inline std::string_view extractPath(std::string_view path) const override {
VELOX_CHECK_EQ(path.find(scheme()), 0, "");
const auto filePath = path.substr(scheme().length());
const auto filePath =
(path.find(scheme()) == 0) ? path.substr(scheme().length()) : path;
return getFileSystem(filePath, config_)->extractPath(filePath);
}

Expand Down
16 changes: 8 additions & 8 deletions velox/dwio/common/CacheInputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,14 @@ bool CacheInputStream::loadFromSsd(
MicrosecondTimer timer(&ssdLoadUs);
file.load(ssdPins, pins);
} catch (const std::exception& e) {
LOG(ERROR) << "IOERR: Failed SSD loadSync " << entry.toString() << ' '
<< e.what() << process::TraceContext::statusLine()
<< fmt::format(
"stream region {} {}b, start of load {} file {}",
region_.offset,
region_.length,
region.offset - region_.offset,
fileIds().string(fileNum_));
// LOG(ERROR) << "IOERR: Failed SSD loadSync " << entry.toString() << ' '
// << e.what() << process::TraceContext::statusLine()
// << fmt::format(
// "stream region {} {}b, start of load {} file {}",
// region_.offset,
// region_.length,
// region.offset - region_.offset,
// fileIds().string(fileNum_));
// Remove the non-loadable entry so that next access goes to storage.
file.erase(cache::RawFileCacheKey{fileNum_, region.offset});
pin_ = std::move(pins[0]);
Expand Down
7 changes: 6 additions & 1 deletion velox/exec/fuzzer/CacheFuzzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "velox/common/memory/MmapAllocator.h"
#include "velox/dwio/common/CachedBufferedInput.h"
#include "velox/exec/tests/utils/TempDirectoryPath.h"
#include "velox/vector/fuzzer/Utils.h"

DEFINE_int32(steps, 10, "Number of plans to generate and test.");

Expand Down Expand Up @@ -498,6 +497,12 @@ void CacheFuzzer::go() {
FLAGS_steps > 0 || FLAGS_duration_sec > 0,
"Either --steps or --duration_sec needs to be greater than zero.");

// O_DIRECT requires I/O size to be the same as a disk file block size which
// is not handled in SSD cache. Misalignment can lead to EINVAL in some
// filesystem and kernel version.
//
// TODO: add this support if needed later.
FLAGS_ssd_odirect = false;
auto startTime = std::chrono::system_clock::now();
size_t iteration = 0;

Expand Down

0 comments on commit 9a3af71

Please sign in to comment.