Skip to content

Commit 79a5fb0

Browse files
committed
drop late
1 parent ae3d014 commit 79a5fb0

5 files changed

Lines changed: 34 additions & 40 deletions

File tree

‎include/paimon/utils/prefetch_cache_config.h‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ namespace paimon {
3434
/// ReadAheadCache to balance memory usage, I/O efficiency, and latency hiding.
3535
class PAIMON_EXPORT CacheConfig {
3636
public:
37-
/// Returns the maximum allowed size (in bytes) for a single cached range.
38-
/// Defaults to 32 MiB.
37+
/// Returns the maximum allowed size (in bytes) for a single cached range, both for the ranges
38+
/// registered up front and, as the cap, for the ones registered mid-read.
39+
/// Defaults to 8 MiB.
3940
uint64_t GetRangeSizeLimit() const {
4041
return range_size_limit_;
4142
}
@@ -45,19 +46,6 @@ class PAIMON_EXPORT CacheConfig {
4546
range_size_limit_ = range_size_limit;
4647
}
4748

48-
/// Returns the maximum allowed size (in bytes) for a single range registered after the cache
49-
/// was initialized, i.e. for the ranges that only become known while reading.
50-
/// Defaults to 8 MiB.
51-
uint64_t GetLateRangeSizeLimit() const {
52-
return late_range_size_limit_;
53-
}
54-
55-
/// Sets the maximum allowed size (in bytes) for a single range registered after the cache was
56-
/// initialized.
57-
void SetLateRangeSizeLimit(uint64_t late_range_size_limit) {
58-
late_range_size_limit_ = late_range_size_limit;
59-
}
60-
6149
/// Returns the maximum gap size (in bytes) considered mergeable between
6250
/// adjacent ranges. Defaults to 512 KiB.
6351
uint64_t GetHoleSizeLimit() const {
@@ -131,27 +119,24 @@ class PAIMON_EXPORT CacheConfig {
131119
private:
132120
// The defaults are aligned with the reader's request granularity and with
133121
// realistic data file sizes:
134-
// - range_size_limit matches the parquet reader's 32 MiB request blocks
135-
// (Arrow ReadRangeCache's own range limit); a smaller limit cuts entries
136-
// below the request size, so a request can never be served from one piece.
122+
// - range_size_limit bounds a single coalesced cached range, both for the
123+
// ranges registered up front at Init and, as the cap, for the ones
124+
// registered mid-read (see range_split_alignment below). One range is one
125+
// prefetch IO, so a smaller limit spreads a large pass over several
126+
// concurrent requests instead of one long serial one; a read spanning
127+
// several adjacent ranges is still served as one hit, as they are
128+
// contiguous. 8 MiB keeps a just-in-time mid-read pass concurrent while
129+
// staying large enough to amortize each request's round trip.
137130
// - pre_buffer_limit must exceed the LARGEST single read a reader issues
138131
// (coalesced column-chunk reads of ~128 MiB were observed): fetches are
139132
// only dispatched up to this window, so a request reaching past it can
140133
// never be served and falls back to a second fetch of the same bytes.
141-
// - late_range_size_limit bounds the ranges registered mid-read instead of
142-
// at Init: those are fetched only just before they are read, and one range
143-
// is one request, so they are cut smaller than range_size_limit to be
144-
// fetched concurrently rather than in one long request. A read spanning
145-
// several of them is still served, as they are adjacent. It is an upper
146-
// bound only: the size such a round is actually cut at is derived from the
147-
// bytes it registers, see range_split_alignment below.
148134
// - hole_size_limit trades bytes against requests: coalescing across a gap
149135
// reads the gap too, but saves a request, and on remote storage a request
150136
// costs a round trip whatever its size. The limit is therefore well above
151137
// the page-sized gaps a filtered read leaves between the pages it keeps,
152138
// which would otherwise each cost a request of their own.
153-
uint64_t range_size_limit_ = 32 * 1024 * 1024;
154-
uint64_t late_range_size_limit_ = 8 * 1024 * 1024;
139+
uint64_t range_size_limit_ = 8 * 1024 * 1024;
155140
uint64_t hole_size_limit_ = 512 * 1024;
156141
uint64_t pre_buffer_limit_ = 256 * 1024 * 1024;
157142
// A fixed size limit cuts a small round into fewer requests than could be

‎src/paimon/common/utils/read_ahead_cache.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ Result<std::optional<uint64_t>> ReadAheadCache::Impl::AddRanges(std::vector<Byte
192192
PAIMON_ASSIGN_OR_RAISE(
193193
std::vector<ByteRange> new_ranges,
194194
ByteRangeCombiner::CoalesceByteRangesAdaptive(
195-
std::move(ranges), config_.GetHoleSizeLimit(), config_.GetLateRangeSizeLimit(),
195+
std::move(ranges), config_.GetHoleSizeLimit(), config_.GetRangeSizeLimit(),
196196
config_.GetRangeSplitAlignment(), config_.GetRangeSplitConcurrency()));
197197
for (const auto& new_range : new_ranges) {
198198
PAIMON_RETURN_NOT_OK(ValidateValueInRange<int64_t>(new_range.offset, "range offset"));

‎src/paimon/common/utils/read_ahead_cache.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ class PAIMON_EXPORT ReadAheadCache {
164164
/// covering entries by walking a disjoint, offset-ordered list, so of a new range only the
165165
/// part no registered range covers is registered; the overlapping part is dropped, as the
166166
/// round that registered it is already fetching those bytes. The registered part is cut into
167-
/// ranges of at most `late_range_size_limit` bytes, one prefetch IO each, so a large pass is
167+
/// ranges of at most `range_size_limit` bytes, one prefetch IO each, so a large pass is
168168
/// fetched concurrently instead of in one long request.
169169
///
170170
/// @param ranges The byte ranges to register on top of the ranges already registered.

‎src/paimon/common/utils/read_ahead_cache_test.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ CacheConfig TestCacheConfig(uint64_t range_size_limit, uint64_t hole_size_limit,
4545
uint64_t pre_buffer_limit) {
4646
CacheConfig config;
4747
config.SetRangeSizeLimit(range_size_limit);
48-
config.SetLateRangeSizeLimit(range_size_limit);
4948
config.SetHoleSizeLimit(hole_size_limit);
5049
config.SetPreBufferLimit(pre_buffer_limit);
5150
return config;
@@ -621,13 +620,12 @@ TEST(TestReadAheadCache, TestAddRangesInterleavesWithRegisteredRanges) {
621620
ASSERT_EQ(4u, CounterOf(ReadAheadCacheMetrics::LATE_DROPPED_BYTES, &cache));
622621
}
623622

624-
// A range registered mid-read is cut at the late range limit, so it is fetched by several
623+
// A range registered mid-read is cut at the range size limit, so it is fetched by several
625624
// concurrent requests rather than by one long one. The pieces are adjacent, so a read spanning
626625
// them is still served.
627626
TEST(TestReadAheadCache, TestAddRangesSplitsLateRanges) {
628-
CacheConfig config = TestCacheConfig(/*range_size_limit=*/26,
627+
CacheConfig config = TestCacheConfig(/*range_size_limit=*/5,
629628
/*hole_size_limit=*/0, /*pre_buffer_limit=*/1024);
630-
config.SetLateRangeSizeLimit(5);
631629
std::string content = "abcdefghijklmnopqrstuvwxyz";
632630
std::shared_ptr<ReadAheadCache> cache_ptr =
633631
CreateTestFileAndCache("data_file", content, config, {{0, 5}});

‎src/paimon/format/parquet/page_filtered_row_group_reader.cpp‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,15 @@ namespace paimon::parquet {
4545

4646
namespace {
4747

48-
/// Ceiling on the value bytes a leaf may reserve up front from column chunk metadata alone.
49-
/// The estimate it caps is a heuristic over footer fields, which are attacker-controlled and
50-
/// need not describe the pages this read touches, so it must not turn into an unbounded eager
51-
/// allocation. Past this size the builder's doubling is already amortized against a large read.
52-
constexpr int64_t kMaxMetadataValueBytesReservation = std::numeric_limits<int64_t>::max();
48+
/// Multiple of a column chunk's compressed size that a leaf may reserve up front in value bytes.
49+
/// The estimate is a heuristic over footer fields, and `total_uncompressed_size` is a bare claim a
50+
/// forged footer can inflate to make a leaf eagerly allocate gigabytes for a near-empty file.
51+
/// `total_compressed_size` is instead checked against the file length before any read (Arrow's
52+
/// ComputeColumnChunkRange, and GetDataPageLayout below, bound a chunk and its pages by it), so a
53+
/// factor of it is bounded by bytes that physically exist. A leaf decompressing within the factor
54+
/// is reserved in full; a more compressible one is capped and left to the builder's amortized
55+
/// doubling, which is cheap against the I/O and decode of a read that large.
56+
constexpr int64_t kMaxReservationDecompressionFactor = 8;
5357

5458
struct DataPageLayout {
5559
int64_t column_chunk_offset;
@@ -374,6 +378,8 @@ Result<std::shared_ptr<arrow::ChunkedArray>> PageFilteredRowGroupReader::ReadFil
374378
const int64_t reserve_values = effective_ranges.RowCount();
375379
int64_t reserve_value_bytes = 0;
376380
const int64_t chunk_bytes = column_chunk ? column_chunk->total_uncompressed_size() : 0;
381+
const int64_t chunk_compressed_bytes =
382+
column_chunk ? column_chunk->total_compressed_size() : 0;
377383
const int64_t chunk_values = column_chunk ? column_chunk->num_values() : 0;
378384
if (chunk_bytes > 0 && chunk_values > 0 && reserve_values > 0) {
379385
// A heuristic, not a bound. total_uncompressed_size is uncompressed but still
@@ -384,11 +390,16 @@ Result<std::shared_ptr<arrow::ChunkedArray>> PageFilteredRowGroupReader::ReadFil
384390
// including the pages this selection skips, so it misleads when wide values sit in
385391
// skipped pages. Either direction only costs performance — the reservation is a
386392
// hint the builder grows past when short — but they are why the result is capped
387-
// instead of trusted. Fixed-width leaves ignore the byte count entirely.
393+
// instead of trusted, by a factor of the file-bounded compressed size rather than the
394+
// forgeable uncompressed one (see kMaxReservationDecompressionFactor). Fixed-width
395+
// leaves ignore the byte count entirely.
388396
const double avg = static_cast<double>(chunk_bytes) / static_cast<double>(chunk_values);
389397
reserve_value_bytes = std::min(
390398
{SaturatingDoubleToInteger<int64_t>(avg * static_cast<double>(reserve_values)),
391-
chunk_bytes, kMaxMetadataValueBytesReservation});
399+
chunk_bytes,
400+
SaturatingDoubleToInteger<int64_t>(
401+
static_cast<double>(chunk_compressed_bytes) *
402+
static_cast<double>(kMaxReservationDecompressionFactor))});
392403
}
393404

394405
PAIMON_RETURN_NOT_OK(ExecuteSkipReadPattern(col_idx, effective_ranges, effective_total,

0 commit comments

Comments
 (0)