Repository navigation
[fix](paimon) Authenticate deferred metadata reads and normalized HDFS storage - #69247
Merged
Merged
Conversation
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 all six changed paths at 1b60a00f3a64da16cfa6ab443753c9d62d42b304, including FE callers, storage configuration, caches and SDK reads. One P2 finding is reported inline: inherited Kerberos settings without a Doris credential pair can replace the HDFS client's login identity with SIMPLE hadoop. No P0/P1 issue was confirmed. There are no existing inline findings to carry forward and no additional user-provided focus points.
- Goal and proof: actual snapshot/partition/time-travel I/O now enters authentication; valid normalized credentials select Kerberos. The fallback regression needs the inline correction. Tests check the intended routing statically.
- Scope and reuse: focused changes in two production files reuse the existing authentication helper and Hadoop configuration builder.
- Concurrency and locks: no new threads or lock order; UGI synchronization ends before actions, and cache loaders execute outside the key-node monitor. No introduced deadlock found.
- Lifecycle and initialization: statement metadata and connector-owned caches/authenticator keep their existing ownership; TCCL restoration remains in
finally. No new static initialization dependency or retained resource. - Configuration: no new key or dynamic-update requirement. Canonical/XML/raw precedence and connector rebuild behavior were traced; the missing-credential fallback is the reported defect.
- Compatibility: no SPI, symbol, serialized field or storage-format change requiring rolling-upgrade handling.
- Thrift/protobuf: no generated definition changes.
- Parallel paths: both latest-snapshot loaders are covered; all seven time-travel kinds, direct OPTIONS resolution, branches, schema/system paths and existing scan/partition authentication were checked.
- Special conditions and errors: system-table exclusion, empty-table
-1, selector-not-found results and malformed-selector failures remain; authentication exceptions do not publish an empty snapshot into the cache. - Test coverage: reviewed the canonical-authenticator test, 12 snapshot-auth scenarios, recording guards and warm REFRESH regression across existing variants. XML/default Kerberos without a keytab pair is missing coverage; recording tests do not exercise real SDK FileIO/KDC login.
- Expected results: no result file changed; the regression compares ordered runtime observations before/after REFRESH.
- Observability: exception causes, table context and existing fallback warning remain; no new subsystem requiring metrics was identified.
- Transactions and persistence: no EditLog, recovery or failover behavior changed.
- Data writes and atomicity: no write path or commit protocol changed.
- FE/BE variables: no new transmitted field or BE code.
- Performance and memory: the additional Configuration is created during memoized authenticator selection; cache-hit behavior remains. No new asymptotic cost or BE allocation was introduced.
- Other invariants: snapshot identity/visibility semantics remain; no MoW/delete-bitmap changes. The final sweep found no second substantiated issue.
All three reviewers returned NO_NEW_VALUABLE_FINDINGS on the identical final inline set in round 3; all candidates and risk focuses have explicit dispositions, with no unresolved source question or unreviewed changed path. Validation was static only: no builds, tests, live KDC/HDFS or packaged-classloader execution was performed. Author-reported execution was not independently rerun.
Review converged after 3 round(s).
CalvinKirs
previously approved these changes
Oct 19, 2026
Contributor
TPC-H: Total hot run time: 29596 ms |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
TPC-H: Total hot run time: 29770 ms |
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Oct 19, 2026
### What problem does this PR solve? Related PR: apache#69247 Problem Summary: The writer lifecycle regression sampled cached JVM gauges before their 15-second refresh and rejected a recovered transient peak during warm-up. Wait for an idle metrics refresh and require two low-water marks plus latest-phase recovery. Keep the fixed baseline and existing sustained-growth limits. ### Release note Fix false positives in the Paimon writer thread lifecycle regression. ### Check List (For Author) - Test: replay the failing CI samples and sustained-growth controls; run the complete regression twice against the CI build in an isolated local Doris, Spark/Paimon, and MinIO environment. - Behavior changed: recovered transient peaks no longer fail bounded warm-up; sustained thread growth still fails. - Does this need documentation: No; test-only change.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Reviewed all seven changed paths at bce156d8e705cae4ecde59897e636d166e80e660. No new inline findings. The existing P2 storage-identity fallback finding still applies and is deduplicated: effective Kerberos configuration without the principal/keytab pair selects a SIMPLE authenticator. There are no confirmed existing P0/P1 comments to carry forward.
Review converged after two static rounds: both complete-review agents and the additional identity-risk agent returned NO_NEW_VALUABLE_FINDINGS on the frozen set. All risk items were dismissed with source evidence or covered by that existing thread. No additional user focus was supplied.
| Critical checkpoint | Conclusion |
|---|---|
| Goal and proof | The new boundaries enclose deferred latest-snapshot, partition-key and time-travel I/O. Twelve added test invocations cover authentication/cache seams; the known fallback issue remains. |
| Scope and reuse | Two cache loaders and the full selector dispatcher reuse the existing authenticated-read helper. Thread-lifecycle stabilization is a bounded test-only adjustment. |
| Concurrency and locks | Existing query/planning callers and cache single-flight remain. Loaders execute synchronously outside the cache key lock; nested UGI acquisition releases its monitor before callbacks. No new lock ordering or worker handoff found. |
| Lifecycle | Connector-owned authentication/caches and statement-owned pins retain their lifetimes. Lazy volatile publication is unchanged; TCCL restores in finally. No new owned threads/resources or static-initialization dependency. |
| Configuration | Existing normalized HDFS/XML settings are recognized; no setting or dynamic-update contract is added. The fallback edge is covered by the existing P2. |
| Compatibility | Private helper changes only; no SPI surface, serialized metadata or storage-format change. Plugin API baselines need no update. |
| Thrift/protobuf | Untouched; field-ID branch comparison is not applicable. |
| Parallel paths | Latest/schema/partition reads, all seven time-travel kinds, direct OPTIONS SDK calls, branch/tag handling, system guards and scan consumers traced. Unchanged best-effort statistics paths introduce no additional regression here. |
| Conditional logic | System-table guards, not-found behavior and empty snapshot -1 semantics are preserved. Effective-config selection has the already-reported fallback concern. |
| Test coverage | Cache enabled/disabled/hit/invalidation, empty snapshots, authentication failure/recovery and selector kinds are covered by new fakes. Existing MVCC/TCCL tests and the added HMS REFRESH check complement them. |
| Test results | No result files changed. Fake expectations match configured ids; REFRESH compares ordered rows. No build or test was run in this review. |
| Observability and errors | Runtime failures propagate; checked failures retain causes under the metadata error context. Failed loads cannot publish an empty snapshot. Existing cache failure metrics and regression phase logs remain adequate. |
| Transactions/persistence/failover | No edit-log, replay, transaction or persistence protocol changes. |
| Writes/atomicity/crashes | Production edits concern metadata authentication. The writer regression retains its fixed baseline and six validation phases; write/commit semantics are unchanged. |
| FE-BE variables | No new wire fields or variables; existing snapshot options still pass through applySnapshot. |
| Performance/memory | Cache policies and bounded ownership remain. Authentication/configuration is memoized per connector; miss-path wrappers add no extra metadata reads or unbounded allocation. BE memory/delete-bitmap concerns are not implicated. |
| Other risks/core invariants | Snapshot/schema pinning and failure semantics are preserved. The 16-second wait matches the normal 15-second metric refresh, and frozen low-water baselines still reject sustained thread growth. No additional verified bug found. |
Validation limits: this was source inspection only, as required. The author's reported build, live KDC/HDFS, concurrent-identity and regression results were not independently reproduced. The new OPTIONS authentication parameter uses an empty map, so actual FileStore startup-selector I/O is established by static tracing of the enclosing wrapper rather than that fake test. Real credential refresh, filesystem behavior and scheduling remain untested here. There are no unresolved production candidates or uncovered changed paths within the static-review scope.
Review converged after 2 round(s).
Gabriel39
force-pushed
the
fix/paimon-kerberos-metadata
branch
from
October 19, 2026 06:01
bce156d to
c5d8569
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
Rebased onto master The conflict was in Local validation after the rebase:
Updated the description to remove the superseded lifecycle sampling changes. |
Contributor
There was a problem hiding this comment.
Reviewed all six changed files at c5d8569. No new findings beyond the existing review context. Review converged after 1 round: both complete-review agents and the additional authentication-risk reviewer returned NO_NEW_VALUABLE_FINDINGS on the same frozen set; all risk items have verified dispositions.
The existing P2 inherited-UGI/SIMPLE-fallback finding still applies: effective Kerberos configuration without a principal/keytab can produce a non-null SIMPLE authenticator that replaces the inherited identity. Independently revalidated and not reposted. No existing P0/P1 finding remains to carry as a blocking ID. This review does not imply that the existing P2 has been fixed.
Critical checkpoint conclusions:
- Goal and evidence: actual snapshot-cache loads, partition-key loads and the complete time-travel resolver now run inside authentication; normalized storage credentials select the plugin Kerberos authenticator. The existing P2 limits the configuration-selection fix. Added tests provide targeted scope/selection oracles.
- Focus and reuse: changes are confined to two production classes and four test files; the shared metadata-read wrapper is reused.
- Concurrency and locks: shared caches/authenticator and statement-scoped concurrent pins were traced. Cache loaders perform I/O outside the key lock; getUGI releases its monitor before invoking doAs tasks. No introduced lock-order cycle or new executor was found.
- Lifecycle: connector ownership of caches/authenticator and statement ownership of pins are preserved. Nested TCCL scopes restore in finally; no added resource lifecycle or static-initialization dependency.
- Configuration: no new setting; effective normalized/XML/raw values now control the existing storage-auth gate. Existing precedence and connector rebuild lifetime are preserved, with the P2 fallback exception above.
- Compatibility: no SPI/public API, persisted-format or rolling-upgrade protocol change.
- Thrift/protobuf: unchanged; no field-id change.
- Parallel paths: both latest-id loaders, all seven time-travel kinds, schema/system-table paths, separate HMS authentication, catalog flavors and downstream scan authentication were reviewed. Selector/pin values remain unchanged.
- Conditions and errors: system/empty/not-found guards are preserved; failed authenticated loads propagate rather than becoming cached empty snapshots. The effective Kerberos gate still needs the correction tracked in the existing P2.
- Test coverage: JUnit 5 cases cover cache enabled/disabled, invalidation, empty snapshots, auth failure/retry and seven selector kinds; the regression adds a warm-query/REFRESH/read comparison. Real identity and integration limits are stated below.
- Expected results: no golden output files changed; added regression queries use ORDER BY 1,2. Comparing unchanged data before/after REFRESH checks successful access but cannot alone prove invalidation; unit cache cases separately check reload behavior.
- Observability: exception causes and existing cache-load failure accounting remain; the generic wrapper now reports metadata failures. No additional instrumentation requirement identified.
- Transactions and persistence: no EditLog/replay, durable metadata or transaction state change.
- Writes and crash atomicity: no write/commit behavior changed by these metadata-read scopes.
- FE/BE variables: no new transmitted field or variable; existing snapshot/schema coordinates and options are preserved.
- Performance: the added auth executes on live cache loads; cache hits avoid that added work. Nested authentication reuses memoized state and adds no snapshot read or unbounded work.
- Core invariants and other issues: no new BE memory, MoW/delete-bitmap, native visible-version or data-mutation behavior. Paimon snapshot/schema visibility and error propagation were traced without a distinct additional defect.
User focus: no additional focus points were supplied.
Validation limits: static review only, as required; no builds or tests were run. Author-reported runs were not independently reproduced. Recording authDepth tests establish execution scope, not real Hadoop UGI identity, keytab login, packaged child-first classloading, credential expiry, or concurrent cache integration. The new OPTIONS test uses an empty options map; real startup-selector I/O was checked by control-flow inspection and related existing tests. No unresolved code candidate or uncovered changed path remains within this static review scope.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 29357 ms |
Contributor
TPC-DS: Total hot run time: 152636 ms |
Contributor
ClickBench: Total hot run time: 24.66 s |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
Fixed the failing
Locally ran the complete original suite using the exact |
2 of 6 tasks
Gabriel39
force-pushed
the
fix/paimon-kerberos-metadata
branch
from
October 19, 2026 08:32
483c388 to
86cd873
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
Rebase validation (86cd873):
|
Contributor
There was a problem hiding this comment.
Reviewed all eight changed paths at 86cd8732c93d2f1040dd27ab08d44e4b26fcd82f, including the surrounding FE consumers, cache implementation, storage/HMS authenticator selection, and Variant parser. No additional verified finding was identified. The existing P2 inherited-UGI fallback concern still applies: an effective Kerberos configuration without the principal/keytab pair can select a SIMPLE subject and suppress the inherited authentication path. It is already reported and is not duplicated. There are no confirmed existing P0/P1 findings to carry as blocking IDs.
The reviewed execution chain is:
flowchart LR
A[FE statement metadata] --> B[Snapshot miss loader or time-travel resolver]
B --> C[Authenticated callback and pinned TCCL]
C --> D[Paimon snapshot, tag, branch and schema FileIO]
D --> E[Snapshot and schema pin used by planning]
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and evidence | The new scopes enclose the actual deferred snapshot reads, partition-key loads, and all seven time-travel kinds. Recording tests exercise these boundaries; the existing P2 limits the normalized-authentication fix. |
| Scope and clarity | Production changes are focused: select from effective Hadoop configuration, reuse the existing read wrapper, and enclose three deferred-read paths. The Variant changes correct a test expectation without changing parsing or sink behavior. |
| Concurrency and locking | Concurrent statements share catalog caches and the authenticator; pins remain statement-owned. Cache miss loaders run outside the cache key monitor. getUGI() releases its monitor before the callback; no introduced reverse lock order or deadlock was found. |
| Lifecycle and initialization | Authenticator publication uses the existing volatile/synchronized protocol. UGI callbacks and TCCL restoration remain scoped by finally; no new thread, static initializer, resource owner, or close obligation is introduced. Current loader code makes Hadoop parent-first, despite historical separate-UGI comments. |
| Configuration and dynamic behavior | No configuration key is added. Effective storage normalization and XML defaults now influence selection; raw Hadoop overrides retain their existing precedence. Catalog rebuild and TTL behavior are unchanged. The missing-credentials fallback is covered by the existing P2. |
| Compatibility | No shared SPI signature, persisted format, or rolling-upgrade protocol changes. |
| Thrift/protobuf IDs | No gensrc/thrift or gensrc/proto change; release-branch ID comparison is not applicable. |
| Parallel paths | Latest, disabled-cache and cached-miss reads, partition cache keys, snapshot/time/tag/version-ref/incremental/branch/options resolution, schema pins, and system-table exclusions were traced. Existing scan-planning authentication remains in place. |
| Conditional logic | System-table guards, absent-snapshot handling, branch identity, selectors, and schema-coordinate checks preserve their previous semantics. A failed authentication remains an exception rather than an empty snapshot. |
| Test coverage | New JUnit cases cover cache hit/miss/disable/invalidation, genuine empty snapshots, authentication failure and retry, partition-key loads, all time-travel kinds, and normalized storage selection. The HMS regression's REFRESH invalidates the shared table scope before the following query reloads. |
| Changed expected results | The changed id-21 row is consistent with FunctionVariantParse<true> setting the SQL null map for invalid JSON: true, NULL, NULL. The invalid input and other assertions remain. The author reports runner generation of the output; generation was not independently repeated here. |
| Observability and error handling | Runtime failures still propagate; checked authentication failures retain their causes under the accurate metadata error prefix. No additional metric or log is necessary for these wrapper changes. |
| Persistence and failover | No new EditLog state, metadata persistence, transaction transition, or failover protocol. |
| Writes and crash atomicity | Production write/commit logic is unchanged; the write regression only corrects the NULL expectation. No new write atomicity or crash-recovery requirement is introduced. |
| FE/BE variable propagation | No new cross-process variable or serialization field. |
| Performance | Snapshot authentication runs in the live loader, so cache hits retain their fast path. The time-travel wrapper adds no metadata lookup, data-size-dependent work, or executor. No substantiated performance regression found. |
| Other concerns and user focus | No additional user focus was supplied. The confirmed existing P2 is the only remaining identified concern; no new distinct issue was found. |
This was a static-only review: no builds, tests, KDC/HDFS experiments, or source edits were performed. Recording fakes cannot establish real credential/classloader integration; the author's runtime results were not independently reproduced, including the expanded rebased regression suite. The REFRESH equality check exercises reload by the current invalidation code, but is not an external-write freshness test. These are validation limits, with no unresolved static candidate or uncovered changed path.
Review convergence: converged after one round, with both complete-review agents and the separate authentication-risk agent returning NO_NEW_VALUABLE_FINDINGS on the empty new-inline set. All three risk focuses were verified and either dismissed with evidence or matched to the existing P2 thread.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 28766 ms |
Contributor
TPC-DS: Total hot run time: 150631 ms |
Contributor
ClickBench: Total hot run time: 24.19 s |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
CHAR-padding regression follow-up:
|
Contributor
There was a problem hiding this comment.
Reviewed all 10 changed files (26 hunks) at b1ff201. No additional inline findings. The existing P2 identity-fallback thread still applies: effective XML Kerberos without a principal/keytab pair selects a non-null SIMPLE plugin authenticator, replacing inherited authentication. It remains actionable and is not reposted. There are no existing P0/P1 findings to carry as blocking IDs.
Converged in 1 round: both complete-scope reviewers and the separate authentication-risk reviewer returned NO_NEW_VALUABLE_FINDINGS on the same finding set. Every candidate and initial risk was independently checked and either deduplicated or dismissed with evidence; no unresolved source-level question or uncovered changed path remains. No additional user focus was supplied.
The inspected invocation flow is:
statement metadata -> snapshot cache miss -> authenticated loader
-> table resolution -> actual snapshot FileIO -> statement schema pin
explicit time travel -> authenticated resolver
-> snapshot / timestamp / tag / branch / incremental / options FileIO
| Critical checkpoint | Conclusion |
|---|---|
| Goal and test evidence | The wrappers now include the actual deferred reads. Recording tests cover authenticated seams; the existing P2 limits credential-free configurations. |
| Scope, clarity and reuse | Two localized production changes reuse the schema-authentication helper; regression corrections match existing SQL contracts. |
| Concurrency and locks | Concurrent cache loading remains single-flight with fenced publication. Remote loader I/O occurs outside key-node locks; getUGI releases its monitor before doAs. No new thread or lock order was introduced. |
| Lifecycle and initialization | Statement metadata, catalog caches, lazy authenticator ownership, refresh invalidation and TCCL finally-restoration remain intact; no new retained resource or static initialization dependency. |
| Configuration and dynamic changes | No new setting. Canonical HDFS/XML values now reach the existing authenticator selector; refresh/rebuild semantics are unchanged. |
| Compatibility and rolling upgrades | No public SPI, function symbol or persisted-format change requiring migration. |
| Thrift and Protobuf | No definition or field-id changes. |
| Parallel paths | Checked latest-snapshot and partition-key loads, all seven time-travel kinds, latest/exact schemas, column handles, system-table guards and partition bypass/filter paths. |
| Special conditions | Existing empty-table -1 and system-table no-MVCC guards remain. Raw Hadoop overrides retain their precedence; storage and HMS identity selection stay separate. |
| Test coverage | Twelve new snapshot-test invocations cover TTL 0/60, invalidation, empty snapshots, authentication failure/recovery and seven selectors. The added canonical authenticator assertion and warm-to-cold REFRESH regression address the intended wiring. |
| Changed test results | Four zero-count expectations match CreateMap's complex-key rejection. Variant row 21 matches the BE ERROR_TO_NULL SQL-null map. Direct Arrow-backed complex-key/NaN normalization tests remain. All output labels match suite queries. |
| Observability and errors | Runtime failures propagate; checked failures retain their cause. Failed loads are not published as empty snapshots. Existing diagnostic context is retained; no new hot logging or metrics are needed. |
| Transactions and persistence | No EditLog, commit, recovery or failover algorithm changes; snapshot/schema fencing is preserved. |
| Data writes and crashes | Production writer/atomicity logic is unchanged; write changes are regression expectations only. No new owned resource introduces a crash-cleanup obligation. |
| FE/BE variable propagation | No new transmitted field or variable. |
| Performance | Authentication is inside deferred cache loaders, preserving cache hits. Explicit time travel gains one outer scope; no new data-sized scan or allocation. |
| Core invariants and other issues | Snapshot visibility/selection is preserved; no BE allocation or MoW/delete-bitmap changes. Further checks of nesting, direct OPTIONS I/O, cold handles and failed cache publication found no additional issue. |
Static review only: no builds, tests or source modifications were performed. Live KDC/HDFS identities, deployed child-classloader behavior, author-reported integration runs and regression-output generation provenance were not independently exercised. These are validation limits; the existing P2 is not claimed fixed.
Review converged after 1 round(s).
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
### What problem does this PR solve? Related PR: apache#69101, apache#69083 Problem Summary: The Paimon CHAR-padding suite assumes STRUCT MAP keys reach the writer, but SQL analysis now rejects complex MAP keys. Assert that rejection for all four complex-key groups and verify the tables remain empty. Keep primitive-key padding/collision checks and the remaining write/DML coverage. Direct complex-key normalization remains covered by PaimonWriteSchemaTest. ### Release note None ### Check List (For Author) - Test: Reproduced the original complete suite failure using the failing CI runtime. Regenerated expected output with the regression runner; an independent normal run passed the complete CHAR-padding and Variant-error suites with zero failures or skips. - Behavior changed: No product behavior changes. - Does this need documentation: No.
Gabriel39
force-pushed
the
fix/paimon-kerberos-metadata
branch
from
October 19, 2026 13:40
b1ff201 to
6e8c052
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
Rebase validation (master
|
Contributor
There was a problem hiding this comment.
Reviewed all eight changed paths at 6e8c0525bdac77d296cc63d3e4eaf5d00c047999. No additional findings to post. The existing P2 identity-fallback thread remains applicable and is not duplicated. No existing P0/P1 finding requires carry-forward.
Checkpoint conclusions:
- Goal and evidence: the actual latest-snapshot cache loads, partition-key snapshot loads, and complete time-travel resolution now enter authentication. Effective Hadoop configuration recognizes normalized HDFS credentials. Added unit tests cover those boundaries; the existing P2 remains the identified exception.
- Scope and reuse: production changes reuse the existing authenticated-read helper and extract the time-travel switch without changing selector semantics. The regression corrections reflect the current analyzer restriction.
- Concurrency and locks: concurrent query threads share connector caches and the memoized authenticator. Existing volatile publication and synchronization remain intact. Snapshot miss loaders execute synchronously outside cache publication locks; waiters share the result. Failed loads complete exceptionally before value publication. No new thread or lock order is introduced.
- Lifecycle: connector-owned authentication and caches retain their existing lifetime, while metadata pins belong to the statement. Refresh invalidates the existing caches, and catalog reconstruction replaces configuration/authentication state. Nested authentication restores TCCL in
finally; no new ownership cycle or static initialization dependency was found. Current FE Hadoop classes are parent-first, as documented in the plugin manager and Paimon POM; older comments about separate Hadoop class copies are historical. - Configuration: no new key is added. Canonical storage settings and explicit Hadoop overrides retain their existing precedence. HMS credentials remain separately resolved and applied at the metastore boundary.
- Compatibility: no public SPI, persisted storage format, or wire contract changes; the helper rename is private.
- Thrift/protobuf: no definitions change, so cross-branch field-ID checks do not apply.
- Parallel paths: reviewed both snapshot-cache call sites, all seven time-travel kinds, schema reads, bound system-table reads, and downstream partition/scan consumers. The connector metadata boundary is shared by cloud and shared-nothing deployments.
- Special conditions: system-table early returns perform no snapshot I/O. Empty tables retain the
-1pin, and authentication/read errors propagate rather than becoming an empty snapshot. The expanded effective-config gate is covered by the existing P2 disposition. - Test coverage: JUnit 5 recording fakes cover TTL enabled/disabled, invalidation, empty snapshots, authentication failure/retry, partition keys, and time-travel kinds. Existing schema, selector, and TCCL tests supplement the new wiring tests. The HMS regression adds an ordered before/after REFRESH comparison under the selected catalog.
- Changed results: all four complex-key INSERT groups fail the existing primitive-MAP-key analyzer check before reaching the writer, so their empty-table count outputs are correct. Direct writer unit tests retain complex-key padding, collision, nested-map, and NaN coverage. Primitive-key and broader DML regression checks remain present.
- Observability and errors: runtime exceptions retain their original type and cause; checked failures retain their cause under the metadata-read message. Existing cache failure statistics and diagnostics cover these operations; no additional metric requirement was identified.
- Transactions and persistence: no EditLog, transaction, replay, or failover implementation changes.
- Writes and crash behavior: production edits affect metadata authentication only. No write commit, rollback, atomicity, or crash-recovery algorithm changes.
- FE/BE propagation: no new transmitted variable or serialization path.
- Performance and memory: cached snapshot hits avoid the new loader scope; misses wrap already-required metadata I/O. No added metadata scan, unbounded allocation, background executor, or native memory ownership was found. Existing nested authentication reuses the memoized authenticator.
- Additional concerns and user focus: no additional user focus was supplied. The final overlooked-path checks found no distinct new issue, uncovered changed path, or unresolved code candidate.
Validation limits: static review only, as required by this invocation. No builds, tests, KDC/HDFS/HMS integration, plugin packaging, or concurrency stress runs were performed. Checked-in output values were reviewed against code and fixture setup; output-generation provenance and the author's reported runtime results were not independently reproduced.
Review converged after 2 round(s).
Contributor
TPC-H: Total hot run time: 28917 ms |
Contributor
TPC-DS: Total hot run time: 150745 ms |
Contributor
ClickBench: Total hot run time: 24.43 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
CalvinKirs
approved these changes
Oct 20, 2026
4 of 14 tasks
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?
Problem Summary:
Queries against Kerberos-protected Paimon storage can fail with
Failed to find latest snapshot idandClient cannot authenticate via: [TOKEN, KERBEROS]. Resolving a Table under authentication does not cover snapshot I/O performed after the resolver returns. In addition, storage configured withhdfs.authentication.*can initialize without a plugin-side Kerberos authenticator because the selection checks only the raw legacy Hadoop key.Authenticate actual snapshot-cache loads, partition-cache key loads, and the complete time-travel resolution. Reuse the metadata-read wrapper and select the storage authenticator from the effective Hadoop configuration, including normalized storage properties. HMS and storage identities remain separate; cache behavior and empty-table snapshot semantics are preserved.
The CHAR-padding regression asserts SQL rejection of complex MAP keys, matching the upstream analyzer guard, while retaining primitive-key padding/collision and DML coverage. The Variant NULL fixture correction is already supplied by master through #69261.
Release note
Fix Kerberos-authenticated Paimon snapshot reads and recognize normalized HDFS authentication settings when selecting the plugin storage identity.
Check List (For Author)
Validation:
test_paimon_write_variant_errorssuite failure on the originalc5d8569a085DEBUG/ASAN artifact with isolated Spark 4/Paimon 1.3.1 and object storage. Regenerated the output with the regression runner and reran the full suite: one passed, zero failures/fatal errors/skips.1eb427f19eb: Paimon moduleinstallwithPaimon*tests: 823 tests, zero failures/errors, one skipped live-connectivity test. Checkstyle and plugin packaging passed.Rebase validation (master
41a58afba2e):install: 832 tests, zero failures/errors, one live-connectivity skip; Checkstyle and plugin packaging passed.CHAR-padding regression follow-up:
86cd8732c93. Regenerated expected output with the regression runner, then independently ran the completetest_paimon_write_char_paddingandtest_paimon_write_variant_errorssuites: two passed, zero failures/fatal errors/skips. Only local endpoints were adapted; product binaries were unchanged.Check List (For Reviewer who merge this PR)