Skip to content

Commit 6db401f

Browse files
mrdrivingduckcodex
andcommitted
perf(fs): avoid redundant object-store metadata requests
Reuse file sizes already obtained during read planning, so object-store-backed reads do not need an extra metadata request. Clarify known-size opening semantics and keep both file-system opening overloads available on concrete implementations. Record a follow-up to reuse known deletion-vector index sizes. Co-authored-by: GPT-5.6 Terra <codex@users.noreply.github.com>
1 parent f75a464 commit 6db401f

18 files changed

Lines changed: 162 additions & 77 deletions

include/paimon/fs/file_system.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,25 @@ class PAIMON_EXPORT FileSystem {
193193
/// failure (e.g., file not found, permission denied).
194194
virtual Result<std::unique_ptr<InputStream>> Open(const std::string& path) const = 0;
195195

196+
/// Open an existing regular file for reading with a known file size.
197+
/// @param path The file path to open.
198+
/// @param file_size The exact size of the file in bytes. It must be non-negative; zero is
199+
/// valid for an empty file.
200+
/// @return Result containing a unique pointer to `InputStream` on success, or error status on
201+
/// failure (e.g., invalid file size, file not found, permission denied).
202+
/// @note File systems may rely on `file_size` to skip metadata requests. The caller must
203+
/// provide the size of an existing regular file and must not expect this method to
204+
/// validate the path, file type, or size. A stale or incorrect size, or a file removed
205+
/// after planning, can cause reads to end early or fail when read instead of failing at
206+
/// open time. Wrapping file systems should forward both `Open` overloads.
207+
virtual Result<std::unique_ptr<InputStream>> Open(const std::string& path,
208+
int64_t file_size) const {
209+
if (file_size < 0) {
210+
return Status::Invalid("file size must be non-negative");
211+
}
212+
return Open(path);
213+
}
214+
196215
/// Create a new file for writing.
197216
/// @param path The file path to create.
198217
/// @param overwrite If true, overwrite existing file; if false, fail if file exists.

src/paimon/common/file_index/bitmap/apply_bitmap_index_batch_reader_test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ class ApplyBitmapIndexBatchReaderTest : public ::testing::Test,
9393
ASSERT_OK_AND_ASSIGN(
9494
file_batch_reader,
9595
PrefetchFileBatchReaderImpl::Create(
96-
/*data_file_path=*/"DUMMY", &reader_builder, fs_, prefetch_batch_count,
97-
batch_size, prefetch_batch_count * 2,
96+
/*data_file_path=*/"DUMMY", /*data_file_size=*/0, &reader_builder, fs_,
97+
prefetch_batch_count, batch_size, prefetch_batch_count * 2,
9898
/*enable_adaptive_prefetch_strategy=*/false, executor_,
9999
/*initialize_read_ranges=*/true,
100100
/*prefetch_cache_mode=*/PrefetchCacheMode::ALWAYS, CacheConfig(), pool_));

src/paimon/common/fs/file_system_test.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,20 @@ TEST_P(FileSystemTest, TestSimpleWriteAndRead) {
283283
ASSERT_OK(in_stream->Close());
284284
}
285285

286+
TEST_P(FileSystemTest, TestOpenWithKnownFileSize) {
287+
const std::string content = "abcdefghijk";
288+
const std::string file_path = test_root_ + "/file.data";
289+
ASSERT_OK(fs_->WriteFile(file_path, content, /*overwrite=*/true));
290+
291+
ASSERT_OK_AND_ASSIGN(auto input_stream,
292+
fs_->Open(file_path, static_cast<int64_t>(content.size())));
293+
ASSERT_OK_AND_ASSIGN(int64_t file_size, input_stream->Length());
294+
ASSERT_EQ(file_size, content.size());
295+
ASSERT_OK(input_stream->Close());
296+
297+
ASSERT_TRUE(fs_->Open(file_path, /*file_size=*/-1).status().IsInvalid());
298+
}
299+
286300
TEST_P(FileSystemTest, TestWriteMultipleTimes) {
287301
std::vector<std::string> content_vec = {"abc", "defg", "hi", "j", "k"};
288302
std::string content = "abcdefghijk";

src/paimon/common/fs/object_store_file_system.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,17 @@ Result<std::unique_ptr<InputStream>> ObjectStoreFileSystem::Open(const std::stri
388388
ToUri(object_path), metadata.value().size);
389389
}
390390

391+
Result<std::unique_ptr<InputStream>> ObjectStoreFileSystem::Open(const std::string& path,
392+
int64_t file_size) const {
393+
PAIMON_RETURN_NOT_OK(ValidateValueNonNegative(file_size, "file size"));
394+
PAIMON_ASSIGN_OR_RAISE(ObjectStorePath object_path, ParsePath(path));
395+
if (object_path.key.empty()) {
396+
return Status::Invalid(fmt::format("{} is a directory", path));
397+
}
398+
return std::make_unique<ObjectStoreInputStream>(client_, read_ahead_limiter_, object_path,
399+
ToUri(object_path), file_size);
400+
}
401+
391402
Result<std::unique_ptr<FileStatus>> ObjectStoreFileSystem::GetFileStatus(
392403
const std::string& path) const {
393404
PAIMON_ASSIGN_OR_RAISE(ObjectStorePath object_path, ParsePath(path));

src/paimon/common/fs/object_store_file_system.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ class PAIMON_EXPORT ObjectStoreFileSystem : public FileSystem {
8585
~ObjectStoreFileSystem() override = default;
8686

8787
Result<std::unique_ptr<InputStream>> Open(const std::string& path) const override;
88+
Result<std::unique_ptr<InputStream>> Open(const std::string& path,
89+
int64_t file_size) const override;
8890
Result<std::unique_ptr<FileStatus>> GetFileStatus(const std::string& path) const override;
8991
Status ListDir(const std::string& directory,
9092
std::vector<std::unique_ptr<BasicFileStatus>>* file_status_list) const override;

src/paimon/common/fs/object_store_file_system_test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ TEST(ObjectStoreFileSystemTest, TestOpenBucketRootIsDirectory) {
159159
ASSERT_EQ(client->list_calls_, 0);
160160
}
161161

162+
TEST(ObjectStoreFileSystemTest, TestOpenWithKnownLengthSkipsHead) {
163+
auto client = std::make_shared<MockObjectStoreClient>();
164+
client->objects_["file"] = "data";
165+
ObjectStoreFileSystem fs("s3", client);
166+
ASSERT_OK_AND_ASSIGN(auto stream, fs.Open("s3://bucket/file", 4));
167+
ASSERT_EQ(stream->Length().value(), 4);
168+
ASSERT_EQ(client->head_calls_, 0);
169+
}
170+
162171
TEST(ObjectStoreFileSystemTest, TestPathWithLeadingSlashes) {
163172
auto client = std::make_shared<MockObjectStoreClient>();
164173
client->objects_["file"] = "data";

src/paimon/common/fs/resolving_file_system.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ Result<std::unique_ptr<InputStream>> ResolvingFileSystem::Open(const std::string
7979
return fs->Open(path);
8080
}
8181

82+
Result<std::unique_ptr<InputStream>> ResolvingFileSystem::Open(const std::string& path,
83+
int64_t file_size) const {
84+
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<FileSystem> fs, GetRealFileSystem(path));
85+
return fs->Open(path, file_size);
86+
}
87+
8288
Result<std::unique_ptr<OutputStream>> ResolvingFileSystem::Create(const std::string& path,
8389
bool overwrite) const {
8490
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<FileSystem> fs, GetRealFileSystem(path));

src/paimon/common/fs/resolving_file_system.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ class ResolvingFileSystem : public FileSystem {
4141
~ResolvingFileSystem() override = default;
4242

4343
Result<std::unique_ptr<InputStream>> Open(const std::string& path) const override;
44+
Result<std::unique_ptr<InputStream>> Open(const std::string& path,
45+
int64_t file_size) const override;
4446
Result<std::unique_ptr<OutputStream>> Create(const std::string& path,
4547
bool overwrite) const override;
4648

src/paimon/common/reader/prefetch_file_batch_reader_impl.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ std::pair<int64_t, int64_t> ComputeBatchSliceByReadRange(
5656
} // namespace
5757

5858
Result<std::unique_ptr<PrefetchFileBatchReaderImpl>> PrefetchFileBatchReaderImpl::Create(
59-
const std::string& data_file_path, const ReaderBuilder* reader_builder,
59+
const std::string& data_file_path, int64_t data_file_size, const ReaderBuilder* reader_builder,
6060
const std::shared_ptr<FileSystem>& fs, uint32_t prefetch_max_parallel_num, int32_t batch_size,
6161
uint32_t prefetch_batch_count, bool enable_adaptive_prefetch_strategy,
6262
const std::shared_ptr<Executor>& executor, bool initialize_read_ranges,
@@ -83,16 +83,17 @@ Result<std::unique_ptr<PrefetchFileBatchReaderImpl>> PrefetchFileBatchReaderImpl
8383

8484
std::shared_ptr<ReadAheadCache> cache;
8585
if (prefetch_cache_mode != PrefetchCacheMode::NEVER) {
86-
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InputStream> input_stream, fs->Open(data_file_path));
86+
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<InputStream> input_stream,
87+
fs->Open(data_file_path, data_file_size));
8788
cache = std::make_shared<ReadAheadCache>(input_stream, cache_config, pool);
8889
}
8990
std::vector<std::future<Result<std::unique_ptr<FileBatchReader>>>> futures;
9091
for (uint32_t i = 0; i < prefetch_max_parallel_num; i++) {
9192
futures.push_back(Via(executor.get(),
92-
[&fs, &data_file_path, &reader_builder,
93+
[&fs, &data_file_path, data_file_size, &reader_builder,
9394
&cache]() -> Result<std::unique_ptr<FileBatchReader>> {
9495
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<InputStream> input_stream,
95-
fs->Open(data_file_path));
96+
fs->Open(data_file_path, data_file_size));
9697
auto cache_input_stream = std::make_shared<CacheInputStream>(
9798
std::move(input_stream), cache);
9899
return reader_builder->Build(cache_input_stream);

src/paimon/common/reader/prefetch_file_batch_reader_impl.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@ class Metrics;
5656
class PrefetchFileBatchReaderImpl : public PrefetchFileBatchReader {
5757
public:
5858
static Result<std::unique_ptr<PrefetchFileBatchReaderImpl>> Create(
59-
const std::string& data_file_path, const ReaderBuilder* reader_builder,
60-
const std::shared_ptr<FileSystem>& fs, uint32_t prefetch_max_parallel_num,
61-
int32_t batch_size, uint32_t prefetch_batch_count, bool enable_adaptive_prefetch_strategy,
62-
const std::shared_ptr<Executor>& executor, bool initialize_read_ranges,
63-
PrefetchCacheMode prefetch_cache_mode, const CacheConfig& cache_config,
64-
const std::shared_ptr<MemoryPool>& pool);
59+
const std::string& data_file_path, int64_t data_file_size,
60+
const ReaderBuilder* reader_builder, const std::shared_ptr<FileSystem>& fs,
61+
uint32_t prefetch_max_parallel_num, int32_t batch_size, uint32_t prefetch_batch_count,
62+
bool enable_adaptive_prefetch_strategy, const std::shared_ptr<Executor>& executor,
63+
bool initialize_read_ranges, PrefetchCacheMode prefetch_cache_mode,
64+
const CacheConfig& cache_config, const std::shared_ptr<MemoryPool>& pool);
6565

6666
~PrefetchFileBatchReaderImpl() override;
6767

0 commit comments

Comments
 (0)