Skip to content

Commit b6813d2

Browse files
author
zhongzheyun
committed
perf(manifest): selectively decode bucket entries with bounded Avro indexes
Probe manifest versions and buckets before materializing selected entries when the input bytes are retained in memory. Keep single-pass reads for uncached inputs and manifests known to contain only the requested bucket. Support precise Avro bitmap selection and preserve physical row IDs across schema resets. Reuse block positions from complete scans with a bounded index, falling back to sequential skipping when the bound is exceeded. Cover cached schema evolution, invalid versions, cache fallback, sparse selection, compressed blocks, empty inputs and index overflow. Generated-by: OpenAI Codex (GPT-6)
1 parent a5b92a1 commit b6813d2

10 files changed

Lines changed: 559 additions & 65 deletions

‎src/paimon/core/manifest/manifest_file.cpp‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
#include <limits>
2323
#include <utility>
2424

25+
#include "arrow/array/array_primitive.h"
2526
#include "arrow/c/abi.h"
2627
#include "arrow/c/bridge.h"
2728
#include "paimon/common/data/columnar/columnar_row.h"
2829
#include "paimon/common/utils/arrow/status_utils.h"
30+
#include "paimon/common/utils/checked_cast.h"
2931
#include "paimon/core/io/rolling_file_writer.h"
3032
#include "paimon/core/manifest/manifest_entry.h"
3133
#include "paimon/core/manifest/manifest_entry_serializer.h"
@@ -106,7 +108,81 @@ Status ManifestFile::ReadBucketEntries(const std::string& file_name, int32_t buc
106108
entries->push_back(std::move(entry));
107109
}
108110
return Status::OK();
109-
});
111+
},
112+
[this, bucket](FileBatchReader* reader) { return PrepareBucketRead(reader, bucket); });
113+
}
114+
115+
Status ManifestFile::PrepareBucketRead(FileBatchReader* reader, int32_t bucket) const {
116+
if (!reader->SupportPreciseBitmapSelection()) {
117+
return Status::OK();
118+
}
119+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<ArrowSchema> c_schema, reader->GetFileSchema());
120+
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Schema> file_schema,
121+
arrow::ImportSchema(c_schema.get()));
122+
const auto& target_type = serializer_->GetDataType();
123+
const std::string& version_name = target_type->field(0)->name();
124+
const std::string& bucket_name = target_type->field(3)->name();
125+
auto version_field = file_schema->GetFieldByName(version_name);
126+
auto bucket_field = file_schema->GetFieldByName(bucket_name);
127+
if (!version_field || !bucket_field || version_field->type()->id() != arrow::Type::INT32 ||
128+
bucket_field->type()->id() != arrow::Type::INT32) {
129+
return Status::OK();
130+
}
131+
arrow::FieldVector probe_fields;
132+
for (const auto& field : file_schema->fields()) {
133+
if (field->name() == version_name || field->name() == bucket_name) {
134+
probe_fields.push_back(field);
135+
}
136+
}
137+
auto probe_schema = arrow::schema(probe_fields);
138+
ArrowSchema probe_c_schema;
139+
PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportSchema(*probe_schema, &probe_c_schema));
140+
PAIMON_RETURN_NOT_OK(
141+
reader->SetReadSchema(&probe_c_schema, /*predicate=*/nullptr, std::nullopt));
142+
RoaringBitmap32 selected;
143+
bool row_ids_fit = true;
144+
while (true) {
145+
PAIMON_ASSIGN_OR_RAISE(BatchReader::ReadBatch batch, reader->NextBatch());
146+
if (BatchReader::IsEofBatch(batch)) {
147+
break;
148+
}
149+
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
150+
std::shared_ptr<arrow::Array> array,
151+
arrow::ImportArray(batch.first.get(), batch.second.get()));
152+
if (!array || array->type_id() != arrow::Type::STRUCT) {
153+
return Status::Invalid("Manifest bucket probe must return a struct array");
154+
}
155+
auto rows = checked_pointer_cast<arrow::StructArray>(array);
156+
auto version_array = rows->GetFieldByName(version_name);
157+
auto bucket_array = rows->GetFieldByName(bucket_name);
158+
if (!version_array || !bucket_array || version_array->type_id() != arrow::Type::INT32 ||
159+
bucket_array->type_id() != arrow::Type::INT32) {
160+
return Status::Invalid("Manifest bucket probe must return int32 version and bucket");
161+
}
162+
auto versions = checked_pointer_cast<arrow::Int32Array>(version_array);
163+
auto buckets = checked_pointer_cast<arrow::Int32Array>(bucket_array);
164+
for (int64_t i = 0; i < rows->length(); ++i) {
165+
if (rows->IsNull(i) || versions->IsNull(i) || buckets->IsNull(i)) {
166+
return Status::Invalid("Manifest version and bucket must not be null");
167+
}
168+
// Validate every entry, including buckets not selected by this scan.
169+
PAIMON_RETURN_NOT_OK(ManifestEntrySerializer::ValidateVersion(versions->Value(i)));
170+
if (buckets->Value(i) == bucket) {
171+
PAIMON_ASSIGN_OR_RAISE(uint64_t file_row, reader->GetPreviousBatchFileRowId(i));
172+
if (file_row > std::numeric_limits<uint32_t>::max()) {
173+
row_ids_fit = false;
174+
} else {
175+
selected.Add(static_cast<uint32_t>(file_row));
176+
}
177+
}
178+
}
179+
}
180+
// Keep the on-disk schema; ManifestMetaReader still performs schema evolution afterwards.
181+
ArrowSchema full_c_schema;
182+
PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportSchema(*file_schema, &full_c_schema));
183+
return reader->SetReadSchema(
184+
&full_c_schema, /*predicate=*/nullptr,
185+
row_ids_fit ? std::optional<RoaringBitmap32>(std::move(selected)) : std::nullopt);
110186
}
111187

112188
Result<std::vector<ManifestFileMeta>> ManifestFile::Write(

‎src/paimon/core/manifest/manifest_file.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ class ManifestFile : public ObjectsFile<ManifestEntry> {
6767
std::vector<ManifestEntry>* entries) const;
6868

6969
private:
70+
Status PrepareBucketRead(FileBatchReader* reader, int32_t bucket) const;
71+
7072
ManifestFile(const std::shared_ptr<FileSystem>& file_system,
7173
const std::shared_ptr<ReaderBuilder>& reader_builder,
7274
const std::shared_ptr<WriterBuilder>& writer_builder,

‎src/paimon/core/manifest/manifest_file_test.cpp‎

Lines changed: 140 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
#include <utility>
2626

2727
#include "arrow/api.h"
28+
#include "arrow/ipc/json_simple.h"
2829
#include "gtest/gtest.h"
2930
#include "paimon/common/data/binary_row.h"
3031
#include "paimon/common/data/data_define.h"
3132
#include "paimon/core/io/data_file_meta.h"
3233
#include "paimon/core/manifest/file_kind.h"
3334
#include "paimon/core/manifest/file_source.h"
3435
#include "paimon/core/manifest/manifest_entry.h"
36+
#include "paimon/core/manifest/manifest_entry_serializer.h"
3537
#include "paimon/core/manifest/manifest_file_meta.h"
3638
#include "paimon/core/stats/simple_stats.h"
3739
#include "paimon/core/utils/file_store_path_factory.h"
@@ -40,6 +42,8 @@
4042
#include "paimon/defs.h"
4143
#include "paimon/format/file_format.h"
4244
#include "paimon/format/file_format_factory.h"
45+
#include "paimon/format/format_writer.h"
46+
#include "paimon/format/writer_builder.h"
4347
#include "paimon/fs/local/local_file_system.h"
4448
#include "paimon/memory/memory_pool.h"
4549
#include "paimon/testing/utils/binary_row_generator.h"
@@ -104,30 +108,45 @@ class ManifestFileTest : public testing::Test {
104108
const std::string& file_format_str, const std::string& root_path,
105109
const std::string& file_name, const std::shared_ptr<MemoryPool>& pool,
106110
const std::optional<int32_t>& bucket = std::nullopt) const {
111+
EXPECT_OK_AND_ASSIGN(
112+
std::vector<ManifestEntry> entries,
113+
TryReadManifestEntry(file_format_str, root_path, file_name, pool, bucket));
114+
return entries;
115+
}
116+
117+
Result<std::vector<ManifestEntry>> TryReadManifestEntry(
118+
const std::string& file_format_str, const std::string& root_path,
119+
const std::string& file_name, const std::shared_ptr<MemoryPool>& pool,
120+
const std::optional<int32_t>& bucket = std::nullopt, bool cache_enabled = false) const {
107121
std::shared_ptr<FileSystem> file_system = std::make_shared<LocalFileSystem>();
108-
EXPECT_OK_AND_ASSIGN(std::shared_ptr<FileFormat> file_format,
109-
FileFormatFactory::Get(file_format_str, {}));
122+
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<FileFormat> file_format,
123+
FileFormatFactory::Get(file_format_str, {}));
110124
auto unused_schema = arrow::schema(arrow::FieldVector({arrow::field("f0", arrow::utf8())}));
111-
EXPECT_OK_AND_ASSIGN(std::shared_ptr<FileStorePathFactory> path_factory,
112-
FileStorePathFactory::Create(
113-
root_path, unused_schema, /*partition_keys=*/{},
114-
/*default_part_value=*/"", file_format->Identifier(),
115-
/*data_file_prefix=*/"data-",
116-
/*legacy_partition_name_enabled=*/true, /*external_paths=*/{},
117-
/*global_index_external_path=*/std::nullopt,
118-
/*index_file_in_data_file_dir=*/false, pool));
119-
EXPECT_OK_AND_ASSIGN(CoreOptions options,
120-
CoreOptions::FromMap({{Options::FILE_FORMAT, "orc"}}));
121-
EXPECT_OK_AND_ASSIGN(
125+
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<FileStorePathFactory> path_factory,
126+
FileStorePathFactory::Create(
127+
root_path, unused_schema, /*partition_keys=*/{},
128+
/*default_part_value=*/"", file_format->Identifier(),
129+
/*data_file_prefix=*/"data-",
130+
/*legacy_partition_name_enabled=*/true, /*external_paths=*/{},
131+
/*global_index_external_path=*/std::nullopt,
132+
/*index_file_in_data_file_dir=*/false, pool));
133+
PAIMON_ASSIGN_OR_RAISE(CoreOptions options,
134+
CoreOptions::FromMap({{Options::FILE_FORMAT, "orc"}}));
135+
if (cache_enabled) {
136+
options.WithCache(
137+
std::make_shared<CountingRoutingCache>(CacheKind::MANIFEST, 64 * 1024 * 1024));
138+
}
139+
PAIMON_ASSIGN_OR_RAISE(
122140
std::unique_ptr<ManifestFile> manifest_file,
123141
ManifestFile::Create(file_system, file_format, "zstd", path_factory,
124142
/*target_file_size=*/1024, pool, options, unused_schema));
125143
std::vector<ManifestEntry> manifest_entries;
126144
if (bucket) {
127-
EXPECT_OK(
145+
PAIMON_RETURN_NOT_OK(
128146
manifest_file->ReadBucketEntries(file_name, bucket.value(), &manifest_entries));
129147
} else {
130-
EXPECT_OK(manifest_file->Read(file_name, /*filter=*/nullptr, &manifest_entries));
148+
PAIMON_RETURN_NOT_OK(
149+
manifest_file->Read(file_name, /*filter=*/nullptr, &manifest_entries));
131150
}
132151

133152
return manifest_entries;
@@ -373,45 +392,104 @@ TEST_F(ManifestFileTest, TestReadBucketEntriesSkipsDeserializingOtherBuckets) {
373392
"manifest-3a44a0da-1008-463c-914e-28d271375e24-0", pool);
374393
ASSERT_EQ(2, source_entries.size());
375394

376-
auto test_dir = UniqueTestDirectory::Create();
377-
ASSERT_TRUE(test_dir);
378-
std::shared_ptr<FileSystem> file_system = test_dir->GetFileSystem();
379-
ASSERT_OK(file_system->Mkdirs(FileStorePathFactory::ManifestPath(test_dir->Str())));
380-
ASSERT_OK_AND_ASSIGN(std::shared_ptr<FileFormat> file_format,
381-
FileFormatFactory::Get("avro", {}));
382-
auto unused_schema = arrow::schema(arrow::FieldVector({arrow::field("f0", arrow::utf8())}));
383-
ASSERT_OK_AND_ASSIGN(
384-
std::shared_ptr<FileStorePathFactory> path_factory,
385-
FileStorePathFactory::Create(test_dir->Str(), unused_schema, /*partition_keys=*/{},
386-
/*default_part_value=*/"", file_format->Identifier(),
387-
/*data_file_prefix=*/"data-",
388-
/*legacy_partition_name_enabled=*/true, /*external_paths=*/{},
389-
/*global_index_external_path=*/std::nullopt,
390-
/*index_file_in_data_file_dir=*/false, pool));
391-
ASSERT_OK_AND_ASSIGN(CoreOptions options,
392-
CoreOptions::FromMap({{Options::FILE_FORMAT, "orc"}}));
393-
ASSERT_OK_AND_ASSIGN(
394-
std::unique_ptr<ManifestFile> manifest_file,
395-
ManifestFile::Create(file_system, file_format, "zstd", path_factory,
396-
/*target_file_size=*/1024, pool, options, unused_schema));
397-
398-
ManifestEntry invalid_other_bucket(FileKind(static_cast<int8_t>(2)),
399-
source_entries[0].Partition(), /*bucket=*/1,
400-
/*total_buckets=*/2, source_entries[0].File());
401-
ManifestEntry valid_target_bucket(FileKind::Add(), source_entries[1].Partition(), /*bucket=*/0,
402-
/*total_buckets=*/2, source_entries[1].File());
403-
using WrittenFile = std::pair<std::string, int64_t>;
404-
ASSERT_OK_AND_ASSIGN(
405-
WrittenFile written_file,
406-
manifest_file->WriteWithoutRolling({invalid_other_bucket, valid_target_bucket}));
395+
// A cache that rejects manifest keys must fall back to the single-pass reader.
396+
for (int32_t cache_mode : {0, 1, 2}) {
397+
SCOPED_TRACE(cache_mode);
398+
auto test_dir = UniqueTestDirectory::Create();
399+
ASSERT_TRUE(test_dir);
400+
auto file_system = std::make_shared<CountingFileSystem>();
401+
ASSERT_OK(file_system->Mkdirs(FileStorePathFactory::ManifestPath(test_dir->Str())));
402+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<FileFormat> file_format,
403+
FileFormatFactory::Get("avro", {}));
404+
auto unused_schema = arrow::schema(arrow::FieldVector({arrow::field("f0", arrow::utf8())}));
405+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<FileStorePathFactory> path_factory,
406+
FileStorePathFactory::Create(
407+
test_dir->Str(), unused_schema, /*partition_keys=*/{},
408+
/*default_part_value=*/"", file_format->Identifier(),
409+
/*data_file_prefix=*/"data-",
410+
/*legacy_partition_name_enabled=*/true, /*external_paths=*/{},
411+
/*global_index_external_path=*/std::nullopt,
412+
/*index_file_in_data_file_dir=*/false, pool));
413+
ASSERT_OK_AND_ASSIGN(CoreOptions options,
414+
CoreOptions::FromMap({{Options::FILE_FORMAT, "orc"}}));
415+
if (cache_mode != 0) {
416+
options.WithCache(std::make_shared<CountingRoutingCache>(
417+
cache_mode == 1 ? CacheKind::MANIFEST : CacheKind::DEFAULT, 64 * 1024 * 1024));
418+
}
419+
ASSERT_OK_AND_ASSIGN(
420+
std::unique_ptr<ManifestFile> manifest_file,
421+
ManifestFile::Create(file_system, file_format, "null", path_factory,
422+
/*target_file_size=*/1024, pool, options, unused_schema));
407423

408-
std::vector<ManifestEntry> all_entries;
409-
ASSERT_NOK_WITH_MSG(manifest_file->Read(written_file.first, /*filter=*/nullptr, &all_entries),
410-
"Unsupported byte value 2 for file kind.");
424+
ManifestEntry invalid_other_bucket(FileKind(static_cast<int8_t>(2)),
425+
source_entries[0].Partition(), /*bucket=*/1,
426+
/*total_buckets=*/2, source_entries[0].File());
427+
ManifestEntry valid_target_bucket(FileKind::Add(), source_entries[1].Partition(),
428+
/*bucket=*/0,
429+
/*total_buckets=*/2, source_entries[1].File());
430+
using WrittenFile = std::pair<std::string, int64_t>;
431+
ASSERT_OK_AND_ASSIGN(
432+
WrittenFile written_file,
433+
manifest_file->WriteWithoutRolling({invalid_other_bucket, valid_target_bucket}));
434+
435+
// Exercise both a cold cache and reuse of the retained manifest bytes.
436+
for (int32_t read = 0; read < 2; ++read) {
437+
std::vector<ManifestEntry> bucket_entries;
438+
ASSERT_OK(manifest_file->ReadBucketEntries(written_file.first, /*bucket=*/0,
439+
&bucket_entries));
440+
ASSERT_EQ(std::vector<ManifestEntry>({valid_target_bucket}), bucket_entries);
441+
ASSERT_EQ(cache_mode == 1 ? 1 : read + 1, file_system->open_count);
442+
}
443+
std::vector<ManifestEntry> missing_bucket_entries;
444+
ASSERT_OK(manifest_file->ReadBucketEntries(written_file.first, /*bucket=*/2,
445+
&missing_bucket_entries));
446+
ASSERT_TRUE(missing_bucket_entries.empty());
447+
std::vector<ManifestEntry> all_entries;
448+
ASSERT_NOK_WITH_MSG(
449+
manifest_file->Read(written_file.first, /*filter=*/nullptr, &all_entries),
450+
"Unsupported byte value 2 for file kind.");
451+
}
452+
}
411453

412-
std::vector<ManifestEntry> bucket_entries;
413-
ASSERT_OK(manifest_file->ReadBucketEntries(written_file.first, /*bucket=*/0, &bucket_entries));
414-
ASSERT_EQ(std::vector<ManifestEntry>({valid_target_bucket}), bucket_entries);
454+
TEST_F(ManifestFileTest, TestBucketProbeValidatesVersionsOutsideSelectedBucket) {
455+
auto pool = GetDefaultPool();
456+
ManifestEntrySerializer serializer(pool);
457+
auto type =
458+
arrow::struct_({arrow::field(serializer.GetDataType()->field(0)->name(), arrow::int32()),
459+
arrow::field(serializer.GetDataType()->field(3)->name(), arrow::int32())});
460+
const std::vector<std::pair<std::string, std::string>> cases = {
461+
{R"([[999,1],[2,0]])", "Unsupported version: 999"},
462+
{R"([[1,1],[2,0]])", "not compatible"},
463+
{R"([[null,1],[2,0]])", "must not be null"},
464+
{R"([[2,null],[2,0]])", "must not be null"}};
465+
for (const auto& item : cases) {
466+
SCOPED_TRACE(item.first);
467+
auto dir = UniqueTestDirectory::Create();
468+
ASSERT_TRUE(dir);
469+
auto fs = dir->GetFileSystem();
470+
const std::string manifest_dir = FileStorePathFactory::ManifestPath(dir->Str());
471+
ASSERT_OK(fs->Mkdirs(manifest_dir));
472+
const std::string file_name = "manifest-invalid-version";
473+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<FileFormat> format,
474+
FileFormatFactory::Get("avro", {}));
475+
ArrowSchema c_schema;
476+
ASSERT_TRUE(arrow::ExportSchema(*arrow::schema(type->fields()), &c_schema).ok());
477+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<WriterBuilder> builder,
478+
format->CreateWriterBuilder(&c_schema, 2));
479+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<OutputStream> output,
480+
fs->Create(manifest_dir + "/" + file_name, false));
481+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<FormatWriter> writer, builder->Build(output, "null"));
482+
auto array = arrow::ipc::internal::json::ArrayFromJSON(type, item.first).ValueOrDie();
483+
ArrowArray c_array;
484+
ASSERT_TRUE(arrow::ExportArray(*array, &c_array).ok());
485+
ASSERT_OK(writer->AddBatch(&c_array));
486+
ASSERT_OK(writer->Flush());
487+
ASSERT_OK(writer->Finish());
488+
ASSERT_OK(output->Close());
489+
ASSERT_NOK_WITH_MSG(TryReadManifestEntry("avro", dir->Str(), file_name, pool, /*bucket=*/0,
490+
/*cache_enabled=*/true),
491+
item.second);
492+
}
415493
}
416494

417495
TEST_F(ManifestFileTest, TestLegacyManifestFormatIsReadOnly) {
@@ -539,6 +617,11 @@ TEST_F(ManifestFileTest, TestManifestFileCompatibleWithJavaPaimon09) {
539617
ASSERT_EQ(expected_manifest_entries,
540618
ReadManifestEntry("avro", paimon::test::GetDataDir() + "/avro", "avro_manifest_09",
541619
pool, /*bucket=*/0));
620+
ASSERT_OK_AND_ASSIGN(std::vector<ManifestEntry> cached_entries,
621+
TryReadManifestEntry("avro", paimon::test::GetDataDir() + "/avro",
622+
"avro_manifest_09", pool, /*bucket=*/0,
623+
/*cache_enabled=*/true));
624+
ASSERT_EQ(expected_manifest_entries, cached_entries);
542625
}
543626

544627
TEST_F(ManifestFileTest, TestManifestFileCompatibleWithJavaPaimon11) {
@@ -578,6 +661,11 @@ TEST_F(ManifestFileTest, TestManifestFileCompatibleWithJavaPaimon11) {
578661
ASSERT_EQ(expected_manifest_entries,
579662
ReadManifestEntry("avro", paimon::test::GetDataDir() + "/avro", "avro_manifest_11",
580663
pool, /*bucket=*/0));
664+
ASSERT_OK_AND_ASSIGN(std::vector<ManifestEntry> cached_entries,
665+
TryReadManifestEntry("avro", paimon::test::GetDataDir() + "/avro",
666+
"avro_manifest_11", pool, /*bucket=*/0,
667+
/*cache_enabled=*/true));
668+
ASSERT_EQ(expected_manifest_entries, cached_entries);
581669
}
582670

583671
} // namespace paimon::test

0 commit comments

Comments
 (0)