Repository navigation
[test](paimon) Cover the catalog snapshot pin on the Paimon system-table source - #69569
Merged
morningman merged 1 commit intoOct 25, 2026
Merged
Conversation
…ble source ### What problem does this PR solve? Issue Number: None Related PR: apache#68897, apache#69343 Problem Summary: For the Paimon system tables that plan on the BE ($files, $file_indexes, $partitions, $manifests, $statistics, $table_indexes) the FE also sends the system table's source table in doris.serialized-system-source, and the JNI scanner rebuilds the system table from that payload instead of serialized_table. Until apache#69343 only serialized_table carried the catalog snapshot pin (scan.snapshot-id taken from a version-managed catalog's pointer, apache#65867), so in a REST catalog these system tables were planned on the newest snapshot in storage rather than the one the catalog publishes, on every branch, since the side channel was added (apache#66247); the review of apache#68897 pointed this out for chain tables. apache#69343 (the Paimon 2.1.0 upgrade) now prepares that source the way tableForBackend prepares serialized_table: pin the catalog snapshot branch by branch, apply the relation's @options over it, then drop the catalog loader. Its tests cover $file_indexes on a single-branch table. This adds the two cases that are not covered yet: - a scan.fallback-branch pair: each branch of the transported source carries its own catalog pointer and no catalog loader (a chain table nests the same pairs); - an explicit relation snapshot still wins over the catalog pin on the transported source. ### Release note None ### Check List (For Author) - Test: Unit Test - PaimonBackendBoundTableTest: 2 new cases, 38 in all - Without the pin in prepareSystemSource the fallback case fails, and pinning after the relation options makes the snapshot case fail - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed PR #69569 at head 8109fea964893f5f7d1f04d322b7109ea3025c17 using static analysis. No new actionable findings and no existing blocking inline comments were identified. No additional user focus was supplied.
The tests cover the intended FE transport boundary: each fallback branch retains its own catalog snapshot and loses its catalog loader in doris.serialized-system-source, while an explicit relation snapshot overrides the catalog pin. The helper decodes the same source payload that JNI uses to rebuild the system table.
Code-review checkpoint conclusions:
- Goal and proof: the new payload assertions distinguish independent branch pins (2/1) and relation precedence (1 over catalog 2). Test execution was outside this review's scope.
- Scope and reuse: the change is confined to two JUnit tests and a helper that reuses the existing provider, fixture, and deserializer.
- Concurrency and locks: no new concurrency or locking; each test uses its own temporary warehouse and local catalog instances.
- Lifecycle and initialization: existing commit helpers close writers and committers; no new static initialization, shared mutable fixture, or lifetime dependency is introduced.
- Configuration: no production configuration is added or changed.
- Compatibility: no production API, function symbol, storage format, or upgrade behavior changes.
- Thrift/protobuf: no definitions change.
- Parallel paths: traced both serialized-table and system-source construction, branch-by-branch pinning, loader removal, and JNI reconstruction; existing file-index and chain-source tests cover adjacent reconstruction paths.
- Conditions and errors: missing payloads, wrong pins, lost fallback wrappers, and retained loaders fail assertions or deserialization; the catalog call counter supplements the payload assertion.
- Test coverage: both requested transport cases are covered. Existing neighboring tests address filesystem catalogs, other system tables, incremental options, and failure paths. These two additions do not execute a complete JNI fallback query.
- Expected results: only inline assertions are added; no generated regression results are modified. Expected snapshot values match the fixture pointers and relation options.
- Observability: no runtime path changes requiring new logging or metrics.
- Transactions and persistence: production transaction, EditLog, replay, and failover behavior is unchanged; snapshot writes are confined to test fixtures.
- Data writes and atomicity: test data uses the existing scoped batch-write/commit helper; no production write path changes.
- FE/BE propagation: no new transmitted variable; the helper reads the existing options JSON key consumed by the scanner.
- Performance: two small local fixtures and one additional payload decode per test; no production hot path changes.
- Other issues and core invariants: no substantiated regression in snapshot transport, error propagation, fixture isolation, memory ownership, or production data/version/delete-bitmap behavior was found.
Validation limits: no builds, unit tests, regression tests, Checkstyle execution, or mutation experiments were run. Review coverage includes every changed hunk and the relevant producer/consumer paths; there are no unresolved candidates or unreviewed changed files. Review converged in one round.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 28698 ms |
Contributor
TPC-DS: Total hot run time: 150217 ms |
Contributor
ClickBench: Total hot run time: 24.26 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
924060929
approved these changes
Oct 25, 2026
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.
What problem does this PR solve?
Issue Number: None
Related PR: #68897, #69343
Problem Summary:
In short. This PR only adds tests. Some Paimon system tables are planned on the BE. For them, the JNI scanner rebuilds the system table from the source table that the FE sends in
doris.serialized-system-source. In a version-managed catalog, that payload must be pinned to the snapshot the catalog publishes. #69343 added this pin. This PR adds the two cases its tests do not cover: ascan.fallback-branchpair, and an explicit relation snapshot that must still take precedence over the pin.Background. Six Paimon system tables are planned on the BE:
$files,$file_indexes,$partitions,$manifests,$statisticsand$table_indexes(PaimonScanParams#resolvesSnapshotOnBackend). For these tables the FE sends two payloads:serialized_tableis built bytableForBackend.doris.serialized-system-sourcewas added in [fix](paimon) Harden JNI reader options and transport #66247 so that a smaller BE can cap the manifest parallelism. The FE always sends it when the system table has a source. When it is present,PaimonJniScanner#applyBackendManifestParallelismrebuilds the system table from this payload instead of fromserialized_table.A version-managed catalog (REST) owns the committed snapshot pointer. Once the catalog loader is removed, Paimon falls back to listing the snapshot directory, and that directory can already contain a snapshot the catalog has not published. #65867 therefore pins
scan.snapshot-idto the catalog's pointer (PaimonScanPlanProvider#pinCatalogSnapshot).The problem, and what it cost. Before #69343, only
serialized_tablecarried that pin. The side channel shipped the source unpinned, and the BE rebuilt from the side channel. So in a REST catalog these six system tables were planned on the newest snapshot in storage rather than the published one, on every branch. Inside the publication window,$partitionscould list partitions of a snapshot the catalog had not published yet. The review of #68897 pointed this out for chain tables.#69343 (the Paimon 2.1.0 upgrade) now prepares the side-channel source in the same order as
tableForBackend(prepareSystemSource):@optionsover the pin.Its tests cover
$file_indexeson a single-branch table, on the FE and through the JNI rebuild.How this PR fixes it. Two FE unit tests in
PaimonBackendBoundTableTestread the side channel out ofpaimon.options_json:systemSourcePinsEachFallbackBranchToItsOwnCatalogSnapshotbuilds$partitionsover a fallback pair whose two catalogs point at snapshots 2 and 1. Each branch of the transported source carries its own pointer and no catalog loader. A chain table nests the same pairs.relationSnapshotOutranksTheCatalogPinOnTheSystemSourceuses a catalog that points at snapshot 2 and@options('scan.snapshot-id'='1'). The transported source says 1.Paimon 2.1.0 still has the two limitations documented on
pinCatalogSnapshot:$filesemits its marker splits fromSnapshotReader#partitions.$snapshotsand$bucketsread withoutscan.snapshot-id.So that javadoc stays as written.
Results.
prepareSystemSourcePaimonBackendBoundTableTest@options)$file_indexescase failexpected: <1> but was: <2>)All 847 tests of fe-connector-paimon pass, with 1 skipped as before, and checkstyle passes. No regression suite is added: it would need a version-managed (REST) Paimon catalog whose pointer lags the snapshot directory, and the docker fixtures do not provide one.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)