Skip to content

Commit 368ebad

Browse files
committed
serde/parquet: account stats collector memory in the writer
column_writer::memory_usage() drives the translator's memory reservation but counted only buffered page data. Each leaf column also retains the row group's min/max bounds untruncated, which for byte-array columns grows with the value width.
1 parent e905139 commit 368ebad

4 files changed

Lines changed: 62 additions & 1 deletion

File tree

src/v/serde/parquet/column_stats_collector.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,25 @@ class column_stats_collector {
106106

107107
int64_t null_count() const { return _null_count; }
108108

109+
// Byte-array bounds are retained untruncated.
110+
int64_t memory_usage() const {
111+
return bound_bytes(_min) + bound_bytes(_max);
112+
}
113+
109114
bound_ref_type min() { return normalize(_min, true); }
110115
bound_ref_type max() { return normalize(_max, false); }
111116

112117
private:
118+
static int64_t bound_bytes(const std::optional<value_type>& bound) {
119+
if constexpr (std::is_trivially_copyable_v<value_type>) {
120+
return 0;
121+
} else {
122+
return bound.has_value()
123+
? static_cast<int64_t>(bound->val.size_bytes())
124+
: 0;
125+
}
126+
}
127+
113128
bound_ref_type normalize(bound_ref_type v, bool min) {
114129
if constexpr (std::is_floating_point_v<decltype(v->val)>) {
115130
// min floats are always written as -0 and max as 0

src/v/serde/parquet/column_writer.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,14 @@ class buffered_column_writer final : public column_writer::impl {
314314
}
315315

316316
int64_t memory_usage() const override {
317-
return _total_memory_usage + current_page_memory_usage();
317+
// Memory that flushing will release.
318+
//
319+
// NOTE: the translator flushes to recover from memory pressure, so a
320+
// term it cannot free does not belong here: _file_stats lives until
321+
// close and is not accounted here.
322+
return _total_memory_usage + current_page_memory_usage()
323+
+ _current_page_stats.memory_usage()
324+
+ _flushed_stats.memory_usage();
318325
}
319326

320327
int64_t current_page_memory_usage() const override {

src/v/serde/parquet/tests/column_stats_collector_test.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <cmath>
1919
#include <limits>
2020
#include <optional>
21+
#include <string>
2122

2223
using namespace serde::parquet;
2324
using testing::DoubleEq;
@@ -64,6 +65,31 @@ TEST(ColumnStatsCollector, FloatingPoint) {
6465
EXPECT_THAT(collector.max(), DoubleBound(IsPositiveZero()));
6566
}
6667

68+
TEST(ColumnStatsCollector, MemoryUsage) {
69+
// Fixed-width columns hold their bounds inline: no heap, regardless of the
70+
// values recorded.
71+
column_stats_collector<int64_value, ordering::int64> numeric;
72+
EXPECT_EQ(numeric.memory_usage(), 0);
73+
numeric.record_value(int64_value{42});
74+
numeric.record_value(int64_value{7});
75+
EXPECT_EQ(numeric.memory_usage(), 0);
76+
77+
// Byte-array columns retain the untruncated min and max values, so the
78+
// footprint tracks the size of those bounds.
79+
column_stats_collector<byte_array_value, ordering::byte_array> binary;
80+
EXPECT_EQ(binary.memory_usage(), 0);
81+
auto cat = byte_array_value{iobuf::from("cat")};
82+
binary.record_value(cat);
83+
// min == max == "cat": 3 bytes each.
84+
EXPECT_EQ(binary.memory_usage(), 6);
85+
auto big = byte_array_value{iobuf::from(std::string(4096, 'z'))};
86+
binary.record_value(big);
87+
// min stays "cat" (3 bytes), max is now the 4096-byte value.
88+
EXPECT_EQ(binary.memory_usage(), 3 + 4096);
89+
binary.reset();
90+
EXPECT_EQ(binary.memory_usage(), 0);
91+
}
92+
6793
TEST(ColumnStatsCollector, Binary) {
6894
column_stats_collector<byte_array_value, ordering::byte_array> collector;
6995
auto empty = byte_array_value{iobuf::from("")};

src/v/serde/parquet/tests/writer_test.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,19 @@ TEST(ParquetWriter, FlushesRowGroupWhenSizeExceeded) {
259259
w.close().get();
260260
}
261261

262+
TEST(ParquetWriter, BufferedMemoryZeroAfterFlush) {
263+
iobuf file;
264+
writer w({.schema = simple_schema()}, make_iobuf_ref_output_stream(file));
265+
w.init().get();
266+
for (size_t i = 0; i < 3; ++i) {
267+
w.write_row(make_row(/*data_size=*/64)).get();
268+
}
269+
EXPECT_GT(w.stats().buffered_size, 0);
270+
w.flush_row_group().get();
271+
EXPECT_EQ(w.stats().buffered_size, 0);
272+
w.close().get();
273+
}
274+
262275
TEST(ParquetWriter, ColumnMemoryEstimateCoversActual) {
263276
#ifdef SEASTAR_DEFAULT_ALLOCATOR
264277
GTEST_SKIP() << "memory::stats() reports fixed values under the system "

0 commit comments

Comments
 (0)