[core][rest] Report partition statistics when registering partitions - #9295
Open
sundapeng wants to merge 1 commit into
Open
[core][rest] Report partition statistics when registering partitions#9295sundapeng wants to merge 1 commit into
sundapeng wants to merge 1 commit into
Conversation
sundapeng
force-pushed
the
upstream/s3-report-statistics-when-registering
branch
2 times, most recently
from
August 18, 2026 17:19
61e7393 to
88e01ad
Compare
sundapeng
force-pushed
the
upstream/s3-report-statistics-when-registering
branch
6 times, most recently
from
August 19, 2026 10:30
5983db4 to
f503469
Compare
Contributor
|
Maybe just |
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.
sundapeng
force-pushed
the
upstream/s3-report-statistics-when-registering
branch
from
August 19, 2026 12:48
f503469 to
5ea393c
Compare
Member
Author
|
Done, changed to |
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
2 participants
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
A catalog-managed format table has no snapshot, so it cannot report partition statistics the way a
data table does through
commitSnapshot. This PR carries them on the partition registration:CreatePartitionsRequestgainspartitionStatisticsandreplaceStatistics, so registration andstatistics land in one request and one server-side transaction.
The flag says how a report combines with the stored value, per field:
recordCount,fileSizeInBytes,fileCountandlastFileCreationTime. Anoverwriting commit reports this way (
INSERT OVERWRITE, the partition pinned statically or leftto the data), as does a full rescan, having accounted for the whole partition.
An appending commit reports this way (
INSERT INTO), having accounted only for what it wrote.A field reported as unknown keeps the stored value either way, and statistics never create or remove
a partition row. Version skew in either direction leaves the columns unknown rather than wrong: an
old server ignores both fields, and a client that reports nothing sends neither, so the request it
puts on the wire is the one it sends today. A non-empty adding report is not retry-safe, so it is
never replayed automatically.
This PR only opens the channel. Nothing sends statistics yet.
Tests
HttpClientRetrySafetyTest,RESTApiJsonTest,MockRESTCatalogTest,CatalogFormatTablePartitionManagerTest,CachingCatalogTestandDelegateCatalogTestcover retrysafety, the request shape, both ways of combining end to end against the mock server, unknown
fields, and the new overload forwarding through the catalog wrappers.
API and Format
Catalog.createPartitionsandRESTApi.createPartitionstake the statistics and the flag, foldedinto the
ignoreIfExistsoverloads that #8707 added and nothing yet calls, so no layer grows athird overload. On
FormatTablePartitionManagerthe reporting overload is the one an implementation provides, sonone can report nothing by accident, and the two-argument form defaults onto it. And
PartitionStatisticsModeanswersretrySafe()for itself so a mode added later cannot inherit thetransport retry by omission. No released signature changes, and no format change.
supportsPartitionModificationkeeps its behaviour and gets an accurate javadoc: it gates thePartitionModificationhandler a table is given, not the catalog methods themselves, which a RESTcatalog registering Format Table partitions while reporting false had already made plain.
Documentation
Both fields are in
docs/static/rest-catalog-open-api.yaml.