[WIP][core] Report what a format table commit wrote to the catalog - #9296
Open
sundapeng wants to merge 2 commits into
Open
[WIP][core] Report what a format table commit wrote to the catalog#9296sundapeng wants to merge 2 commits into
sundapeng wants to merge 2 commits into
Conversation
This was referenced Aug 18, 2026
sundapeng
force-pushed
the
upstream/s4-commit-reports-what-it-wrote
branch
9 times, most recently
from
August 19, 2026 11:09
8dac356 to
7d151be
Compare
A catalog-managed format table registers the partitions a commit wrote. It has no snapshot, so the channel data tables use to report statistics — commitSnapshot — does not exist for it, and there is nowhere else for the numbers to go. This adds them to the registration itself. CreatePartitionsRequest gains an optional partitionStatistics list and a replaceStatistics flag. Registration and statistics then land in one request and one server-side transaction, which is the point: "the statistics failed but the partition registered" is not a state anyone has to handle, and no extra round trip is paid for the numbers. The alternative, a separate endpoint, buys only the ability to report statistics without registering — which is exactly the state worth not having. Rather than grow a third overload on each layer, the reporting arguments fold into the ignoreIfExists overloads that apache#8707 added and nothing yet calls. Catalog keeps the long-standing two-argument createPartitions plus the reporting one, and RESTApi is back to a single method; CachingCatalog, DelegateCatalog and RESTCatalog each lose a forwarder. No released signature changes. On FormatTablePartitionManager the reporting overload is the one an implementation provides and the two-argument form defaults onto it, rather than the other way round. An implementation that overrode only the short form would otherwise drop every report and leave the caller no way to notice, which is a rule the compiler can keep and a javadoc line cannot. Its twenty-five test doubles each spell the two arguments they ignore; that is the price, and the production implementation loses a forwarder in exchange. While here, supportsPartitionModification says in its javadoc that createPartitions and alterPartitions do nothing when it is false, which stopped being true once a REST catalog started registering the partitions of a Format Table while still reporting false. What the flag actually gates is the PartitionModification handler a table is given, so committing a table maintains no partitions; calling the catalog methods directly is unaffected. The javadoc now says that. The flag says how a report combines with what the server holds, per field: replacing overwrites all four values, adding sums the three counts and keeps the later creation time, since two timestamps do not add. Which one a writer sends follows from what it accounted for: an appending commit (INSERT INTO) adds, since it knows the files it wrote and not the ones the partition already held, and one statement whose writers commit separately reports each writer's share; an overwriting commit (INSERT OVERWRITE, the partition pinned statically or left to the data) replaces, since it left the partition holding what it wrote and nothing else. Reporting a field as unknown leaves the stored value alone either way, so a reporter that can measure only some fields never erases the others: a report here is read on the observation plane of PartitionStatistics, where a negative field was never measured, and not on the delta plane where it would be a decrement to subtract. Statistics never create or remove a partition row, whatever the numbers say. Compatibility runs both ways and neither direction errors. A new client against an old server: the fields are ignored, the statistics are dropped and the columns stay unknown. The client is not told, because the response carries no acknowledgement — an observation that is missing is a thing consumers can handle, an observation that is wrong is not, and an acknowledgement would be a second contract to keep in step with the first. An old client against a new server: no statistics arrive and the columns stay unknown. The two-argument createPartitions keeps sending exactly the request it sends today, so a caller that reports nothing does not change the shape the server sees. A non-empty adding report declares itself unsafe to retry, so an automatically replayed POST cannot count the same increment twice. A replacing report lands on the same value twice and keeps its retry, as does a request that reports nothing at all. Tests: HttpClientRetrySafetyTest covers which requests declare themselves unsafe to retry — only a non-empty adding report does — and that the flag itself never reaches the wire. RESTApiJsonTest covers the request shape, including that a client reporting nothing sends neither field. MockRESTCatalogTest covers reporting against the mock server end to end: adding accumulating, replacing overwriting, unknown fields leaving the stored value alone, statistics for a partition the request does not register being dropped, and a report that only partly matches not being applied at all. CatalogFormatTablePartitionManagerTest covers what the manager sends: statistics riding in the request of their own partitions, a batch that reports nothing sending an empty list rather than null, and the rejections of malformed reports. CachingCatalogTest and DelegateCatalogTest cover the new overload forwarding through and invalidating the partition cache.
The writer already counted the rows and the bytes, the commit already knows which partitions it wrote, and the catalog now has somewhere to put both. This connects them, behind format-table.commit.report-partition-statistics. Whether a commit replaces or adds follows from what it did to the partition. An appending commit saw only its own files, so it reports an increment: a Flink sink commits once per writer subtask, and N increments over one partition add up to what the job wrote. An overwriting commit replaced everything the partitions held, so what it wrote is the total and it reports that. Static prefix overwrite is why a pure increment cannot express this. Clearing a prefix empties every partition beneath it, including ones this commit writes nothing to; their old data is gone and no increment says so. Those partitions report zero — an exact zero, they really are empty — and ride in the same create request as the written ones, since a statistic can only be reported for a partition its own request registers. Nothing here unregisters a partition, whatever the numbers say. The directories emptied that way come out of the deletion listing, which already had to walk them, so the numbers cost no extra IO: the row count and byte size come from the writer, the file count from the commit messages, and the last file creation time from the commit's own clock. The constructor that predates reporting is gone rather than kept as an overload: its only callers were seven in FormatTableCommitTest, which now pass the flag. The option is off by default. A non-empty adding report makes the create-partitions request unsafe to replay, so a 429 or 503 the client used to retry by itself now surfaces to the caller instead of risking a double-counted increment. That is a change to how an existing write path fails, and it should be opted into rather than inherited. An increment can also drift for reasons no client can see: a writer that is not Paimon, a file deleted out of band. Convergence is a later full report over the same partition, which a follow-up will teach MSCK REPAIR and ANALYZE to produce; a deduplication token in front of every commit would cost more than the drift does. Tests: FormatTableCommitStatisticsTest covers append, dynamic overwrite, static prefix overwrite of a partition this commit does not write, the summation of the independent increments of concurrent writers of one partition, a listing that answers under another scheme, a nested directory in the value-only layout, and reporting turned off.
sundapeng
force-pushed
the
upstream/s4-commit-reports-what-it-wrote
branch
from
August 19, 2026 12:48
7d151be to
d29b0a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
The writer already counts rows and bytes and the commit knows which partitions it wrote, so the
numbers cost no extra IO. This PR reports them along with the partitions a commit registers, behind
format-table.commit.report-partition-statistics.An appending commit adds its numbers to what the catalog holds; an overwriting one replaces them. A static prefix overwrite also empties
partitions it writes nothing to; those report an exact zero and ride in the same create request as
the written ones, since a statistic can only be reported for a partition its own request registers.
Nothing here unregisters a partition, whatever the numbers say.
Off by default: a non-empty adding report is not retry-safe, so a 429 or 503 that used to be retried
transparently now reaches the caller, which is a change in how an existing write path fails. An
increment can also drift when a job retries, and a later full report is what converges it.
Tests
FormatTableCommitStatisticsTest: append, dynamic overwrite, static prefix overwrite of a partitionthis commit does not write, concurrent writers of one partition summing up, another partition
scheme, a nested directory in the value-only layout, and reporting turned off.
API and Format
One new option, off by default.
FormatTableCommittakes the flag as a constructor argument; theconstructor that predates it is dropped rather than kept as an overload, since only its tests called
it. No format change.