This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new c1e522e2491 [fix](be) Decouple Parquet metadata size limit from Thrift
(#67631)
c1e522e2491 is described below
commit c1e522e24918e91039db6010381819b004921b11
Author: Gabriel <[email protected]>
AuthorDate: Thu Sep 10 11:21:24 2026 +0800
[fix](be) Decouple Parquet metadata size limit from Thrift (#67631)
### What problem does this PR solve?
Issue Number: None
Related PR: #25194
Problem Summary: FileScannerV2 reused `thrift_max_message_size` as the
Parquet footer allocation limit, so valid files with metadata larger
than the 100 MiB RPC ceiling were rejected. This PR adds an independent
mutable `parquet_metadata_size_limit` with a 256 MiB default. It
preserves the structural `footer_size <= file_size - 8` validation and
keeps the metadata cap check before allocation and any second read.
### Release note
FileScannerV2 now limits Parquet metadata with the independent
`parquet_metadata_size_limit` configuration, which defaults to 256 MiB.
### Check List (For Author)
- Test: Unit Test
- `NewParquetReaderTest.NativeFooter*` (7 tests passed under ASAN)
- Behavior changed: Yes (valid Parquet metadata between the Thrift RPC
ceiling and the new metadata limit is accepted.)
- Does this need documentation: No
---
be/src/common/config.cpp | 4 ++++
be/src/common/config.h | 2 ++
be/src/format_v2/parquet/parquet_file_context.cpp | 14 +++++-------
be/src/format_v2/parquet/parquet_file_context.h | 3 +--
be/test/format_v2/parquet/parquet_reader_test.cpp | 26 ++++++++++++++++++++---
5 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp
index c62892887cd..5a2d61e0034 100644
--- a/be/src/common/config.cpp
+++ b/be/src/common/config.cpp
@@ -1081,6 +1081,10 @@ DEFINE_mInt32(in_memory_file_size, "1048576"); // 1MB
// Max size of parquet page header in bytes
DEFINE_mInt32(parquet_header_max_size_mb, "1");
+// Max size of parquet file metadata in bytes
+DEFINE_mInt64(parquet_metadata_size_limit, "268435456");
+DEFINE_Validator(parquet_metadata_size_limit,
+ [](const int64_t config) -> bool { return config > 0; });
// Max buffer size for parquet row group
DEFINE_mInt32(parquet_rowgroup_max_buffer_mb, "128");
// Max buffer size for parquet chunk column
diff --git a/be/src/common/config.h b/be/src/common/config.h
index bcdd3b1629f..183c39377d7 100644
--- a/be/src/common/config.h
+++ b/be/src/common/config.h
@@ -1160,6 +1160,8 @@ DECLARE_mInt32(in_memory_file_size);
// Max size of parquet page header in bytes
DECLARE_mInt32(parquet_header_max_size_mb);
+// Max size of parquet file metadata in bytes
+DECLARE_mInt64(parquet_metadata_size_limit);
// Max buffer size for parquet row group
DECLARE_mInt32(parquet_rowgroup_max_buffer_mb);
// Max buffer size for parquet chunk column
diff --git a/be/src/format_v2/parquet/parquet_file_context.cpp
b/be/src/format_v2/parquet/parquet_file_context.cpp
index 8ba8cf94662..108c7f3e56d 100644
--- a/be/src/format_v2/parquet/parquet_file_context.cpp
+++ b/be/src/format_v2/parquet/parquet_file_context.cpp
@@ -109,13 +109,13 @@ Status NativeParquetMetadata::init_schema(bool
enable_mapping_varbinary,
namespace detail {
-Status validate_native_footer_size(uint32_t serialized_size, size_t file_size,
- size_t metadata_size_limit) {
+Status validate_native_footer_size(uint32_t serialized_size, size_t file_size)
{
if (file_size < V2_PARQUET_FOOTER_SIZE ||
serialized_size > file_size - V2_PARQUET_FOOTER_SIZE) {
return Status::Corruption("Parquet v2 footer size {} exceeds file size
{}", serialized_size,
file_size);
}
+ const size_t metadata_size_limit =
static_cast<size_t>(config::parquet_metadata_size_limit);
if (serialized_size > metadata_size_limit) {
return Status::Corruption("Parquet v2 footer size {} exceeds metadata
limit {}",
serialized_size, metadata_size_limit);
@@ -224,13 +224,9 @@ Status parse_native_parquet_footer(io::FileReaderSPtr file,
const uint32_t serialized_size =
decode_fixed32_le(tail.data() + tail.size() -
V2_PARQUET_FOOTER_SIZE);
- // The configured Thrift message ceiling also bounds this file-controlled
allocation. Keep the
- // check before both allocation and the optional second read so a sparse
file cannot force a
- // process-sized metadata buffer merely by advertising a large footer.
- const size_t metadata_size_limit =
- static_cast<size_t>(std::max(config::thrift_max_message_size, 0));
- RETURN_IF_ERROR(
- detail::validate_native_footer_size(serialized_size, file_size,
metadata_size_limit));
+ // Keep the dedicated metadata limit independent of RPC serialization
limits, and enforce it
+ // before allocation so file-controlled footer sizes cannot create
unbounded memory pressure.
+ RETURN_IF_ERROR(detail::validate_native_footer_size(serialized_size,
file_size));
std::vector<uint8_t> serialized_metadata(serialized_size);
if (serialized_size <= tail.size() - V2_PARQUET_FOOTER_SIZE) {
const auto* metadata_start =
diff --git a/be/src/format_v2/parquet/parquet_file_context.h
b/be/src/format_v2/parquet/parquet_file_context.h
index 38e78438e39..327240dbf91 100644
--- a/be/src/format_v2/parquet/parquet_file_context.h
+++ b/be/src/format_v2/parquet/parquet_file_context.h
@@ -82,8 +82,7 @@ namespace detail {
inline constexpr int64_t MAX_SERIALIZED_PARQUET_INDEX_BYTES = 64LL << 20;
-Status validate_native_footer_size(uint32_t serialized_size, size_t file_size,
- size_t metadata_size_limit);
+Status validate_native_footer_size(uint32_t serialized_size, size_t file_size);
std::string build_native_file_cache_key(std::string_view fs_name,
std::string_view path,
int64_t description_mtime, int64_t
reader_mtime,
diff --git a/be/test/format_v2/parquet/parquet_reader_test.cpp
b/be/test/format_v2/parquet/parquet_reader_test.cpp
index 38b6761c89d..508d6811955 100644
--- a/be/test/format_v2/parquet/parquet_reader_test.cpp
+++ b/be/test/format_v2/parquet/parquet_reader_test.cpp
@@ -3203,15 +3203,35 @@ TEST_F(NewParquetReaderTest,
NativeFooterCacheDoesNotReuseMutableUnknownVersion)
EXPECT_EQ(second_profile.get_counter("FileFooterHitCache")->value(), 0);
}
+TEST_F(NewParquetReaderTest,
NativeFooterAcceptsMetadataAboveThriftMessageLimit) {
+ constexpr uint32_t metadata_size = 128UL << 20;
+ constexpr size_t file_size = 512UL << 20;
+ static_assert(metadata_size > 100UL << 20);
+
+ EXPECT_EQ(config::parquet_metadata_size_limit, 256UL << 20);
+ EXPECT_TRUE(
+
format::parquet::detail::validate_native_footer_size(metadata_size,
file_size).ok());
+}
+
TEST_F(NewParquetReaderTest,
NativeFooterSizeIsBoundedBeforeMetadataAllocation) {
- constexpr size_t file_size = 256UL << 20;
- constexpr size_t metadata_limit = 100UL << 20;
+ constexpr size_t file_size = 512UL << 20;
const auto status = format::parquet::detail::validate_native_footer_size(
- static_cast<uint32_t>(metadata_limit + 1), file_size,
metadata_limit);
+ static_cast<uint32_t>(config::parquet_metadata_size_limit + 1),
file_size);
EXPECT_TRUE(status.is<ErrorCode::CORRUPTION>()) << status;
EXPECT_NE(status.to_string().find("metadata limit"), std::string::npos);
}
+TEST_F(NewParquetReaderTest, NativeFooterSizeCannotExceedFileSize) {
+ constexpr size_t file_size = 256UL << 20;
+ EXPECT_TRUE(format::parquet::detail::validate_native_footer_size(
+ static_cast<uint32_t>(file_size - 8), file_size)
+ .ok());
+ const auto status = format::parquet::detail::validate_native_footer_size(
+ static_cast<uint32_t>(file_size - 7), file_size);
+ EXPECT_TRUE(status.is<ErrorCode::CORRUPTION>()) << status;
+ EXPECT_NE(status.to_string().find("file size"), std::string::npos);
+}
+
TEST_F(NewParquetReaderTest, UnknownMtimeUsesPageCacheForImmutableFile) {
_file_path = (_test_dir / "unknown_mtime_page_cache.parquet").string();
write_parquet_file(_file_path);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
