From 33aed19249810a032a721509d01b33c6b4817ad9 Mon Sep 17 00:00:00 2001 From: UnamedRus Date: Wed, 23 Sep 2026 17:29:09 +0300 Subject: [PATCH] Iceberg: reuse the Puffin object metadata across deletion-vector reads Loading one deletion vector issued two or three S3 `HEAD` requests against the same object. `loadDeletionVector` already calls `getObjectMetadata` for the Puffin file to read its etag for the `PuffinFilesCache` key, but then passes only the path down: `readFooterBlobs` and `loadDeletionVectorUncached` each build a fresh `RelativePathWithMetadata` with empty metadata, and `createReadBuffer` fills it in with another `getObjectMetadata` for every buffer it opens. Thread the already-fetched metadata into both helpers instead. `createReadBuffer` issues its own request only when `RelativePathWithMetadata::metadata` is empty, so seeding it removes the redundant requests without changing which bytes are read. The two call sites that run before the etag fetch keep passing `std::nullopt` and behave as before. On a lake scan with cold deletion vectors this is the dominant cost: a profile of one such query recorded 5706 `S3HeadObject` requests against 2833 `PuffinFilesRead`, with about 94 seconds spent in `HEAD` alone. Reusing a single metadata snapshot for every read of the same Puffin file is also more coherent than re-fetching it. Iceberg data and delete files are immutable, so the separate requests could only ever have observed the same object. Co-Authored-By: Claude Opus 5 (1M context) --- .../Iceberg/IcebergDeletionVector.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDeletionVector.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDeletionVector.cpp index 7f77e3f40e3c..2de861d59c80 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDeletionVector.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergDeletionVector.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -99,11 +100,15 @@ void logUndersizedPuffinFilesCacheOnce(LoggerPtr log, size_t max_size_in_bytes, FooterBlobsPtr readFooterBlobs( ObjectStoragePtr object_storage, const String & puffin_path, + const std::optional & puffin_metadata, ContextPtr context, LoggerPtr log, bool disable_filesystem_cache) { - RelativePathWithMetadata puffin_object{puffin_path}; + /// Seeding the metadata matters: `createReadBuffer` issues its own `getObjectMetadata` + /// (an S3 `HEAD`) only when it is empty, so a caller that already has it pays one request + /// for the whole deletion-vector load instead of one per buffer it opens. + RelativePathWithMetadata puffin_object{puffin_path, puffin_metadata}; auto read_settings = context->getReadSettings(); if (disable_filesystem_cache) read_settings.enable_filesystem_cache = false; @@ -124,6 +129,7 @@ FooterBlobsPtr readFooterBlobs( DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVectorUncached( ObjectStoragePtr object_storage, const String & puffin_path, + const std::optional & puffin_metadata, Int64 content_offset, Int64 content_size_in_bytes, const IcebergPathFromMetadata & expected_data_file, @@ -136,7 +142,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVectorUncached( PuffinFilesCache * footer_cache = nullptr, const std::optional & footer_key = {}) { - RelativePathWithMetadata puffin_object{puffin_path}; + RelativePathWithMetadata puffin_object{puffin_path, puffin_metadata}; auto read_settings = context->getReadSettings(); if (disable_filesystem_cache) read_settings.enable_filesystem_cache = false; @@ -163,7 +169,8 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVectorUncached( { footer_owner = footer_cache->getOrSetFooter(*footer_key, [&]() { - return readFooterBlobs(object_storage, puffin_path, context, log, /*disable_filesystem_cache=*/ true); + return readFooterBlobs( + object_storage, puffin_path, puffin_metadata, context, log, /*disable_filesystem_cache=*/ true); }); } else @@ -277,6 +284,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVector( return loadDeletionVectorUncached( object_storage, puffin_path, + std::nullopt, content_offset, content_size_in_bytes, expected_data_file, @@ -311,6 +319,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVector( return loadDeletionVectorUncached( object_storage, puffin_path, + std::nullopt, content_offset, content_size_in_bytes, expected_data_file, @@ -336,6 +345,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVector( return loadDeletionVectorUncached( object_storage, puffin_path, + puffin_object.metadata, content_offset, content_size_in_bytes, expected_data_file, @@ -377,6 +387,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVector( return loadDeletionVectorUncached( object_storage, puffin_path, + puffin_object.metadata, content_offset, content_size_in_bytes, expected_data_file, @@ -397,6 +408,7 @@ DataLakeObjectMetadata::ExcludedRowsPtr loadDeletionVector( return loadDeletionVectorUncached( object_storage, puffin_path, + puffin_object.metadata, content_offset, content_size_in_bytes, expected_data_file,