Uh oh!
There was an error while loading. Please reload this page.
[fix](filesystem) replace throttled kerberos relogin with proactive TGT refresh to fix flaky GSS failure - #64394
Conversation
… proactive TGT refresh
…kerberos login failure
… and provide it to fs plugins via fe-filesystem-spi
hello-stephen
commented
Jun 11, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
CalvinKirs
commented
Jun 11, 2026
run buildall |
CalvinKirs
commented
Jun 12, 2026
/review |
There was a problem hiding this comment.
Automated review result: no blocking code issues found in this PR.
Checkpoint conclusions:
- Goal/test: The change addresses the HDFS filesystem Kerberos relogin throttle by replacing
checkTGTAndReloginFromKeytab()with explicit JAAS keytab login plus proactive TGT refresh at 80% lifetime. The added unit tests cover eager login, fresh-ticket no-op behavior, stale-ticket refresh without throttle, IOException propagation, constructor login failure wrapping, and retry after refresh failure. GitHub-visible COMPILE, CheckStyle, and FE UT checks are passing. - Scope/focus: The patch is focused on moving
KerberosTicketUtilsintofe-foundation, wiring dependencies, and updating the HDFS filesystem authenticator. No additional user-provided review focus was present. - Concurrency/lifecycle: The cached Subject/UGI lifecycle is explicit and stable. Refresh is serialized on the authenticator instance, UGI global configuration remains under the existing process-wide lock, and I did not find a new deadlock or shared-state race distinct from the Trino-style credential swap this code ports.
- Configuration/compatibility: No new user-facing config, thrift protocol, storage format, or rolling-upgrade compatibility concern was introduced. The new
fe-foundationdependency is available tofe-common,fe-core, and filesystem provider runtime loading paths. - Parallel paths: The separate FE common Kerberos authenticator already uses the same proactive refresh utility; no remaining production use of Hadoop's throttled
checkTGTAndReloginFromKeytab()was found. - Error handling: Constructor login failures keep the existing unchecked construction contract, and refresh failures are converted back to the filesystem SPI's checked
IOExceptioncontract. Interrupted actions restore the interrupt flag. - Security model: I read
SECURITY.mdandthreat-model.mdbecause this touches authentication. The affected HDFS/external filesystem path is admin-connected external storage under the model; I did not find a client-facing authentication or RBAC regression. - Observability: Existing login success logging plus refresh logging includes the principal and refresh time; that looks sufficient for this narrow relogin path.
Note: gh pr checks currently shows TeamCity P0 Regression, FE coverage, and performance checks failing, but their logs require TeamCity authentication from this runner. I did not classify those opaque CI failures as code findings; they still need normal CI follow-up before merge.
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
CalvinKirs
commented
Jun 15, 2026
run p0 |
hello-stephen
commented
Jun 15, 2026
FE Regression Coverage ReportIncrement line coverage |
CalvinKirs
commented
Jun 22, 2026
run performance |
1 similar comment
CalvinKirs
commented
Jun 23, 2026
run performance |
CalvinKirs
commented
Jun 23, 2026
run buildall |
hello-stephen
commented
Jun 23, 2026
FE Regression Coverage ReportIncrement line coverage |
CalvinKirs
commented
Jun 25, 2026
run buildall |
hello-stephen
commented
Jun 25, 2026
FE UT Coverage ReportIncrement line coverage `` 🎉 |
CalvinKirs
commented
Jun 29, 2026
run performance |
fe-common now depends on fe-foundation, but the cdc_client build script installed fe-common alone via -pl without -am. fe-foundation is never installed into the local repo (the main FE build uses package), so Maven fell back to remote repos and failed to resolve the internal SNAPSHOT. -am pulls fe-foundation into the reactor and installs it alongside.
CalvinKirs
commented
Jun 29, 2026
run buildall |
hello-stephen
commented
Jun 29, 2026
TPC-H: Total hot run time: 29254 ms |
hello-stephen
commented
Jun 29, 2026
TPC-DS: Total hot run time: 173140 ms |
hello-stephen
commented
Jun 29, 2026
ClickBench: Total hot run time: 25.33 s |
CalvinKirs
commented
Jun 30, 2026
/review |
There was a problem hiding this comment.
Automated review summary for PR 64394.
I reviewed the Kerberos refresh change across the HDFS filesystem plugin, the shared Kerberos ticket helper moved into fe-foundation, the fe-common import/dependency adjustment, the filesystem SPI/module dependency propagation, the new unit tests, and the cdc_client build-script change.
Critical checkpoint conclusions:
- Goal/test proof: the PR replaces Hadoop UGI's throttled keytab relogin in the HDFS filesystem plugin with proactive 80% TGT refresh, and adds focused unit coverage for refresh timing, in-place credential swap, retry after failed refresh, and helper behavior.
- Scope/focus: changes are focused on Kerberos ticket refresh and the required module/build dependency wiring.
- Concurrency/lifecycle: refresh is serialized through synchronized getUGI(), keeps the same Subject/UGI for cached Hadoop clients, and follows the same in-place credential replacement pattern as the referenced Trino implementation.
- Configuration/compatibility: no new user-facing config items or storage/protocol format changes. Existing Kerberos properties continue to drive the authenticator path.
- Parallel paths: the older fe-common Kerberos authenticator now imports the shared helper; HDFS plugin packaging and SPI/fe-foundation classpath behavior were checked.
- Tests/results: new Java unit tests cover the key refresh paths. I did not run FE Maven tests/builds because this checkout lacks thirdparty/installed, thirdparty/installed/bin/protoc, and thrift, and fe/AGENTS.md says to stop FE build when protoc is absent.
- Security/threat model: reviewed SECURITY.md and threat-model.md because the PR touches Kerberos authentication. I found no new in-model security issue or trust-boundary change.
- Observability/performance: refresh logs are lightweight and occur only on login/refresh; no hot-path excessive work beyond the synchronized refresh check.
Subagent conclusions:
- optimizer-rewrite: no optimizer/rewrite or semantic-equivalence findings; convergence round 1 returned NO_NEW_VALUABLE_FINDINGS.
- tests-session-config: no test/session/config/build candidate findings; convergence round 1 returned NO_NEW_VALUABLE_FINDINGS.
- No duplicate candidates or accepted inline comments remained. Proposed final comment set was empty, so this review is summary-only.
No additional user focus was provided.
PR approved by at least one committer and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #62023
Problem Summary:
The regression case
test_two_hive_kerberosfails intermittently with:Root cause: the
KerberosHadoopAuthenticatorinfe-filesystem-hdfs(introduced in #62023) relies on Hadoop'sUserGroupInformation.checkTGTAndReloginFromKeytab(), which has a hard-coded 60s relogin throttle (hasSufficientTimeElapsed). When the TGT expires inside that throttle window, the relogin is silently skipped, the subsequent SASL handshake finds no valid ticket, and the JAAS fallback path tries to prompt onSystem.in— which fails in a server process.Fix: replace the throttled reactive relogin with trino's proactive TGT refresh model (ported 1:1 from trino 435
KerberosAuthentication/CachingKerberosHadoopAuthentication/KerberosTicketUtils):doNotPrompt=true,useKeyTab=true,storeKey=true,isInitiator=true) —doNotPrompt=truemakes theSystem.inprompt path impossible.doAs, if past the refresh point, perform a fresh keytab login (no throttle) and swap the new credentials into the existingSubjectin place, so the cached UGI stays valid.IOExceptionto keep the SPI's checked-exception contract.The vendored
KerberosTicketUtilsis placed infe-foundation(org.apache.doris.foundation.security) and provided to filesystem plugins throughfe-filesystem-spi, which also letsfe-common'sHadoopKerberosAuthenticatordrop itsio.trinoimport.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)