Skip to content

Commit

Permalink
Add a limit on loadQuantum (facebookincubator#10242)
Browse files Browse the repository at this point in the history
Summary:
SsdRun only reserves 23 bits (out of 64 bits) for entry size. loadQuantum larger than that will result in cache error.
Fixing facebookincubator#10098

Pull Request resolved: facebookincubator#10242

Reviewed By: xiaoxmeng

Differential Revision: D58711635

Pulled By: zacw7

fbshipit-source-id: 70327443c21d8c6d1d537c145143d46ad012501d
  • Loading branch information
zacw7 authored and facebook-github-bot committed Jun 18, 2024
1 parent 3810cbb commit c6d7390
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
3 changes: 2 additions & 1 deletion velox/connectors/hive/HiveConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ class HiveConfig {
/// The number of prefetch rowgroups
static constexpr const char* kPrefetchRowGroups = "prefetch-rowgroups";

/// The total size in bytes for a direct coalesce request.
/// The total size in bytes for a direct coalesce request. Up to 8MB load
/// quantum size is supported when SSD cache is enabled.
static constexpr const char* kLoadQuantum = "load-quantum";

/// Maximum number of entries in the file handle cache.
Expand Down
19 changes: 17 additions & 2 deletions velox/dwio/common/CachedBufferedInput.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ class CachedBufferedInput : public BufferedInput {
ioStats_(std::move(ioStats)),
executor_(executor),
fileSize_(input_->getLength()),
options_(readerOptions) {}
options_(readerOptions) {
checkLoadQuantum();
}

CachedBufferedInput(
std::shared_ptr<ReadFileInputStream> input,
Expand All @@ -96,7 +98,9 @@ class CachedBufferedInput : public BufferedInput {
ioStats_(std::move(ioStats)),
executor_(executor),
fileSize_(input_->getLength()),
options_(readerOptions) {}
options_(readerOptions) {
checkLoadQuantum();
}

~CachedBufferedInput() override {
for (auto& load : allCoalescedLoads_) {
Expand Down Expand Up @@ -173,6 +177,17 @@ class CachedBufferedInput : public BufferedInput {
// concerns.
void readRegion(const std::vector<CacheRequest*>& requests, bool prefetch);

// We only support up to 8MB load quantum size on SSD and there is no need for
// larger SSD read size performance wise.
void checkLoadQuantum() {
if (cache_->ssdCache() != nullptr) {
VELOX_CHECK_LE(
options_.loadQuantum(),
1 << cache::SsdRun::kSizeBits,
"Load quantum exceeded SSD cache entry size limit.");
}
}

cache::AsyncDataCache* const cache_;
const uint64_t fileNum_;
const std::shared_ptr<cache::ScanTracker> tracker_;
Expand Down
22 changes: 22 additions & 0 deletions velox/dwio/dwrf/test/CacheInputTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <folly/Random.h>
#include <folly/container/F14Map.h>
#include <folly/executors/IOThreadPoolExecutor.h>
#include "velox/common/base/tests/GTestUtils.h"
#include "velox/common/caching/FileIds.h"
#include "velox/common/file/FileSystems.h"
#include "velox/common/io/IoStatistics.h"
Expand Down Expand Up @@ -830,3 +831,24 @@ TEST_F(CacheTest, noCacheRetention) {
ASSERT_EQ(stats.numEntries, cacheEntries.size());
}
}

TEST_F(CacheTest, loadQuotumTooLarge) {
initializeCache(64 << 20, 256 << 20);
auto fileId = std::make_unique<StringIdLease>(fileIds(), "foo");
auto readFile =
std::make_shared<TestReadFile>(fileId->id(), 10 << 20, nullptr);
auto readOptions = io::ReaderOptions(pool_.get());
readOptions.setLoadQuantum(9 << 20 /*9MB*/);
VELOX_ASSERT_THROW(
std::make_unique<CachedBufferedInput>(
readFile,
MetricsLog::voidLog(),
fileId->id(),
cache_.get(),
nullptr,
0,
nullptr,
executor_.get(),
readOptions),
"Load quantum exceeded SSD cache entry size limit");
}

0 comments on commit c6d7390

Please sign in to comment.