feat: add metrics raw-input-bytes/storage-read-bytes for parquet and fix clean inte test by lucasfang · Pull Request #439 · alibaba/paimon-cpp

lucasfang · 2026-07-21T10:29:30Z

Purpose

Linked issue: close #xxx

This change bundles two independent items:

1. Parquet byte-level read metrics (feature)

Add two counters so callers can observe how many bytes a Parquet read actually consumes:

parquet.read.raw-input-bytes — logical bytes requested by the reader (top-of-stack demand).
parquet.read.storage-read-bytes — physical bytes read from storage.

2. Revert #397 commit internal-retry / dedup (fix)

#397 changed the commit-exception branch in TryCommitOnce from return Status::Invalid(...) to
return false, driving an internal retry loop plus a CheckCommitted/retry_start_snapshot_id
dedup. This introduced flakiness in CleanInteTest.TestDropPartitionAndExpireSnapshotWithIOException
and diverged from Java, which has no equivalent RetryCommitResult type system in this C++ codebase.

This reverts to the pre-#397 behavior:

Remove retry_start_snapshot_id from the TryCommit loop and the TryCommitOnce signature.
Remove the CheckCommitted method (definition and declaration).
Restore the commit-exception branch to
return Status::Invalid("You need call FilterAndCommit to retry commit for exception. ", ...).

Rationale: on a commit exception the outcome is uncertain (an atomic write may have timed out but
actually landed). Retrying internally can produce a duplicate snapshot. Safe retry belongs to the
upper layer via FilterAndCommit, which dedups by CommitIdentifier(); a bare Commit retry has no
such dedup and would create a duplicate. Returning Status::Invalid makes that contract explicit and
keeps guard.Release() so freshly written meta files are not cleaned up.

Tests

TestCommitWithAtomicWriteSnapshotTimeoutAndActuallySucceed restored to its pre-feat(commit): align functionality with java paimon commit #397 assertions:
the initial Commit returns an error (ASSERT_NOK) while the snapshot has physically landed, and
recovery goes through FilterAndCommit, which idempotently filters the already-committed
identifier (num_committed == 0).
CleanInteTest.TestDropPartitionAndExpireSnapshotWithIOException — re-enabled (removed the
DISABLED_ prefix); the CI-friendly randomized loop step is kept. Verified deterministically by
running the loop exhaustively (i += 1, 0..1000) locally: PASSED (~14 min), confirming the revert
root-causes and eliminates the flakiness rather than lowering its probability.
Parquet metric wiring covered by updated unit tests: parquet_file_batch_reader_test.cpp,
page_filtered_row_group_reader_test.cpp, predicate_pushdown_test.cpp (Create call sites now pass
the raw_input_bytes counter).

API and Format

No public API change: all touched declarations are internal (src/paimon/...); no headers under
include/paimon/ are modified.
No storage format or protocol change.
New observable metric counter keys: parquet.read.raw-input-bytes,
parquet.read.storage-read-bytes.
Internal signature changes: ParquetFileBatchReader::Create/constructor gain a required
raw_input_bytes parameter; FileStoreCommitImpl::TryCommitOnce drops retry_start_snapshot_id
and CheckCommitted is removed. All call sites are updated in this change.

Documentation

No new user-facing feature documentation. The change adds observability counters and reverts an
internal retry behavior; no doc pages are introduced.

Generative AI tooling

…fix clean inte test

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds byte-level Parquet read metrics (raw-input-bytes / storage-read-bytes) by plumbing a shared counter from the Arrow input stream adapter into the Parquet batch reader, and adjusts commit/IT behavior around uncertain commit outcomes.

Changes:

Track and surface Parquet read byte metrics via ArrowInputStreamAdapter → ParquetFileBatchReader → ParquetMetrics.
Update Parquet-related unit tests to pass the new parameter and assert the new metrics.
Change commit retry behavior on uncertain commit failures and update affected integration/unit tests.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/inte/clean_inte_test.cpp	Re-enables an integration test previously disabled for IO exception scenarios.
src/paimon/format/parquet/predicate_pushdown_test.cpp	Updates Parquet reader creation to pass the new raw-bytes counter parameter.
src/paimon/format/parquet/parquet_reader_builder.h	Extracts raw input byte counter from stream adapter and passes it into the batch reader.
src/paimon/format/parquet/parquet_format_defs.h	Defines new Parquet metrics keys for byte-level read tracking.
src/paimon/format/parquet/parquet_file_batch_reader_test.cpp	Wires raw-bytes counter through tests and adds assertions for the new metrics.
src/paimon/format/parquet/parquet_file_batch_reader.h	Extends API to accept a raw-bytes counter and sets the new metrics on `GetReaderMetrics()`.
src/paimon/format/parquet/parquet_file_batch_reader.cpp	Implements new constructor/creator signature to store the raw-bytes counter.
src/paimon/format/parquet/page_filtered_row_group_reader_test.cpp	Updates tests to pass the new raw-bytes counter parameter.
src/paimon/core/operation/file_store_commit_impl_test.cpp	Adjusts expectations for commit failure/uncertain commit and adds `FilterAndCommit` verification.
src/paimon/core/operation/file_store_commit_impl.h	Removes `CheckCommitted` and simplifies `TryCommitOnce` signature.
src/paimon/core/operation/file_store_commit_impl.cpp	Stops internal retry-on-exception path and returns an error instructing caller to use `FilterAndCommit`.
src/paimon/common/utils/arrow/arrow_input_stream_adapter.h	Adds shared atomic counter accessor for raw input bytes.
src/paimon/common/utils/arrow/arrow_input_stream_adapter.cpp	Increments raw input bytes counter on reads (sync and async).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    std::shared_ptr<std::atomic<uint64_t>> raw_input_bytes = raw_input_bytes_;
+    input_stream_->ReadAsync(
+        reinterpret_cast<char*>(buffer->mutable_data()), nbytes, position,
+        [fut, buffer, raw_input_bytes, nbytes](Status callback_status) mutable {
+            if (callback_status.ok()) {
+                if (raw_input_bytes) {
+                    raw_input_bytes->fetch_add(static_cast<uint64_t>(nbytes));
+                }
+                fut.MarkFinished(std::move(buffer));


+        return Status::Invalid("You need call FilterAndCommit to retry commit for exception. ",
+                               commit_result.status().ToString());


+        // commit exception, not sure about the situation and should not clean up the files.
+        PAIMON_LOG_WARN(logger_, "You need call FilterAndCommit to retry commit for exception. %s",
                        commit_result.status().ToString().c_str());


 }

-TEST_F(CleanInteTest, DISABLED_TestDropPartitionAndExpireSnapshotWithIOException) {
+TEST_F(CleanInteTest, TestDropPartitionAndExpireSnapshotWithIOException) {


-    ASSERT_OK_AND_ASSIGN(uint64_t counter,
-                         metrics->GetCounter(CommitMetrics::LAST_COMMIT_ATTEMPTS));
-    ASSERT_EQ(2u, counter);
+    ASSERT_NOK(commit->Commit(msgs, /*commit_identifier=*/1));


zjw1111 · 2026-07-21T11:11:03Z

        const std::map<std::string, std::string>& options, int32_t batch_size,
        std::shared_ptr<::parquet::FileMetaData> file_metadata,
-        const std::shared_ptr<arrow::MemoryPool>& pool);
+        const std::shared_ptr<arrow::MemoryPool>& pool,


make pool at last

feat: add metrics raw-input-bytes/storage-read-bytes for parquet and …

3dfe4b2

…fix clean inte test

Copilot AI review requested due to automatic review settings July 21, 2026 10:29

Copilot AI reviewed Jul 21, 2026

View reviewed changes

zjw1111 reviewed Jul 21, 2026

View reviewed changes

Copilot started reviewing on behalf of lucasfang July 21, 2026 13:17 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add metrics raw-input-bytes/storage-read-bytes for parquet and fix clean inte test#439

feat: add metrics raw-input-bytes/storage-read-bytes for parquet and fix clean inte test#439
lucasfang wants to merge 1 commit into
alibaba:mainfrom
lucasfang:dev3

lucasfang commented Jul 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

zjw1111 Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		return Status::Invalid("You need call FilterAndCommit to retry commit for exception. ",
		commit_result.status().ToString());

Uh oh!

Conversation

lucasfang commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Tests

API and Format

Documentation

Generative AI tooling

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

zjw1111 Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucasfang commented Jul 21, 2026 •

edited

Loading