perf(fs): avoid redundant object-store metadata requests for known-size files

[mrdrivingduck]

I am not sure whether introducing a new FileSystem API is acceptable upstream, but...

Summary

This PR adds an Open(path, file_size) path to the object store file system API for callers that already have trusted file metadata.

Paimon split reads already carry the data-file size from manifest metadata. Passing that size through avoids a per-file object metadata request, while retaining Open(path) as the default path for callers without a known size.

This is a generic object store optimization. My WIP OSS(v2) filesystem uses the supplied size to skip its per-file HeadObject request, and the S3 filesystem can use the same known-size opening path.

Benchmark

The benchmark exercises My WIP OSS(v2) filesystem, using the same real Paimon table throughout:

• Location: oss://paimon-cpp-fs/paimon-cpp-demo-20260710-220136/demo.db/events
• 96 ORC data files, about 4.27 MiB each
• Result validation: count(*) = 786,435, sum(quantity) = 39,713,738

The only difference is how data files are opened:

• Baseline: Open(path), which requires one HeadObject request per file.
• Optimized: Open(path, file_size), which uses the file size already present in the Paimon manifest and skips that request.

Workload	Concurrency / runs	Open(path)	Open(path, size)	Improvement
Create 96 input streams without reading data	Single-threaded, sequential	1019.788 ms	0.120 ms	~8,500× faster; saves 99.988%
Open every file and read its first byte	Single-threaded, 5 alternating runs	1.667 s	1.111 s	33.3% faster
DuckDB full-table aggregate	threads=8, 5 independent processes	1.837 s	1.315 s	28.4% faster
DuckDB full-table aggregate	threads=1, 5 independent processes	3.182 s	2.557 s	19.7% faster

The largest gain occurs when many known-size files are opened sequentially with little or no reading, which isolates the cost of the per-file metadata request. In an end-to-end DuckDB scan, file reads, decoding, and concurrency reduce the relative impact, but the aggregate query still shows a stable 19.7%–28.4% mean latency reduction on this 96-file table.

End-to-end query

SET threads = <1 or 8>;
ATTACH oss://paimon-cpp-fs/paimon-cpp-demo-20260710-220136/ 
  AS paimon_demo (TYPE paimon, READ_ONLY);

SELECT count(*) AS row_count, sum(quantity) AS quantity_sum
FROM paimon_demo.demo.events;

Raw timings from five independent processes:

Mode	Open(path)	Open(path, size)	Median improvement
threads=8	2.478, 1.859, 1.696, 1.896, 1.256 s	1.355, 1.324, 1.254, 1.328, 1.314 s	1.859 → 1.324 s, 28.8% faster
threads=1	3.619, 3.255, 2.682, 3.740, 2.614 s	2.508, 2.695, 2.615, 2.462, 2.503 s	3.255 → 2.508 s, 23.0% faster

[zjw1111]

Thanks for the careful benchmark work here. The direction looks right to me, and it lines up with how Java Paimon threads fileSize through FormatReaderContext down to the format readers, so I don't think a new FileSystem entry point is a problem in principle.

I left three inline notes. The first two (the -1 sentinel and the contract of the new public API) would be good to resolve before merge; the third is a question rather than a request.

One non-blocking follow-up, not for this PR: DeletionVectorsIndexFile::ReadAllDeletionVectors (src/paimon/core/deletionvectors/deletion_vectors_index_file.cpp:53) opens the DV index file by path while it already holds an IndexFileMeta that carries FileSize(), so it could reuse the same known-size path and drop one metadata request per index file. Would you mind leaving a TODO there, or filing it as a later PR? The other Open call sites I looked at (deletion_vector.cpp, file_index_evaluator.cpp) have no trusted total file size available today, so they would need extra metadata plumbing first and are not directly comparable.

[zjw1111]

ObjectStoreFileSystem::Open(path, file_size) starts with ValidateValueNonNegative(file_size, "file size"), so this -1 sentinel makes the legacy overload fail with Status::Invalid("file size -1 is less than 0") on any object-store filesystem, instead of quietly falling back to Open(path). It is not caught today only because every remaining caller of this overload is a test using LocalFileSystem / MockFileSystem, where the base-class default ignores the size — so the sentinel happens to be harmless there and CI stays green.

Instead of defining what a negative size means, could you drop the old Create(data_file_path, reader_builder, ...) overload entirely and migrate the remaining callers (the prefetch reader tests and apply_deletion_vector_batch_reader_test.cpp) to the new one that takes data_file_size? That leaves a single code path and removes the sentinel altogether.

[mrdrivingduck]

Done

[zjw1111]

Could you expand this doc comment to spell out the contract? The neighbouring methods use @param / @return, and a few things are currently left undefined:

• whether the caller must guarantee that file_size matches the object exactly, and what happens when it does not (silently truncated or out-of-range reads deep inside the format reader, rather than a clean error at open time);
• whether a negative value means "size unknown", and whether 0 is a legal value.

The second point matters because the implementations already disagree: this default accepts any value, while ObjectStoreFileSystem::Open rejects negatives. Writing the contract down here would keep future filesystem implementations consistent.

Two smaller things while you are in this header, both optional:

• LocalFileSystem, JindoFileSystem and MockFileSystem declare only the single-argument Open, which hides this overload for anyone holding a concrete type (calls through FileSystem* are fine, which is why everything still compiles). Adding using FileSystem::Open; to those classes would avoid a confusing error later.
• Any decorating filesystem that forwards only Open(path) stays correct but silently loses the optimization. ResolvingFileSystem is handled in this PR; a note in the doc comment that wrappers should forward both overloads would help.

[mrdrivingduck]

Done.

[zjw1111]

A question rather than a request: the single-argument Open falls back to DirectoryExists when HeadObject reports the key as missing, so that opening a common prefix reports "... is a directory" rather than a not-found error. Here the key.empty() check only catches the bucket-root case, and because no HeadObject is issued, a path that is really a directory-like prefix will be accepted and produce a stream that fails only later, during the read.

Is the intent that a caller supplying a trusted file_size can never be pointing at a directory, so the extra check is unnecessary? That seems reasonable to me, I would just like it stated in the API doc. The same reasoning applies to existence: this overload no longer detects a file removed between planning and reading, turning a clean NotExist at open time into a harder-to-read failure mid-read. Also an acceptable trade-off for the optimization, but worth documenting.

[mrdrivingduck]

Done in file_system.h

[mrdrivingduck]

One non-blocking follow-up, not for this PR: DeletionVectorsIndexFile::ReadAllDeletionVectors (src/paimon/core/deletionvectors/deletion_vectors_index_file.cpp:53) opens the DV index file by path while it already holds an IndexFileMeta that carries FileSize(), so it could reuse the same known-size path and drop one metadata request per index file. Would you mind leaving a TODO there, or filing it as a later PR? The other Open call sites I looked at (deletion_vector.cpp, file_index_evaluator.cpp) have no trusted total file size available today, so they would need extra metadata plumbing first and are not directly comparable.

Done with a TODO for DV index. I think it does gain from the optimization in theory, but needs separated benchmark for verification.

[zjw1111]

LGTM. Thanks for addressing all the review points — dropping the legacy Create overload and spelling out the contract of the new Open in the public header both look good to me.

@duanyyyyyyy could you take a look as well? Xinyu mentioned that you had raised a similar question before, so it would be good to hear whether you have any concerns about this approach.

[duanyyyyyyy]

LGTM
In StarRocks, we have the same solution

[lucasfang]

The benchmark case is convincing, and I don't want to hold up the optimization itself. My concern is only the signature, please consider Open(const FileStatus&) instead of Open(path, int64_t), and add a default implementation of FileStatus that callers can construct — see arrow::fs::FileSystem::OpenInputFile(const FileInfo&) for reference.

[mrdrivingduck]

The benchmark case is convincing, and I don't want to hold up the optimization itself. My concern is only the signature, please consider Open(const FileStatus&) instead of Open(path, int64_t), and add a default implementation of FileStatus that callers can construct — see arrow::fs::FileSystem::OpenInputFile(const FileInfo&) for reference.

@lucasfang Good point, thanks. I added a constructible DefaultFileStatus and changed the new open path to take FileStatus instead of separate path and size arguments.

[lucasfang]

Could we make FileStatus a base class with a default implementation (using virtual, but not pure virtual functions)? This would eliminate the need for a separate DefaultFileStatus class.

[mrdrivingduck]

Done

[lucasfang]

+1

[lxy-9602]

+1

[SteNicholas]

LGTM.