Doris-Breakwater commented on issue #66401:
URL: https://github.com/apache/doris/issues/66401#issuecomment-5173063499

   Breakwater-GitHub-Analysis-Slot: slot_b7d4186fad13
   
   Initial maintainer triage, checked against the reported commit `35854e7e92a` 
and its Paimon 1.1.1 dependency:
   
   **Judgment: likely a real authentication-context bug, but the proposed RCA 
is only partially proven.** There is a verified missing authentication boundary 
around the background row-count task, and the production log correlation is 
strong enough to keep this as a bug. However, the source does not establish the 
complete `row-count thread -> unwrapped HMS get_table -> poisoned pool` chain 
as written. The issue is currently unlabeled; an external-catalog/Paimon bug 
label would be appropriate if that matches the project taxonomy.
   
   ### Verified facts
   
   - 
[`ExternalRowCountCache.loadRowCount`](https://github.com/apache/doris/blob/35854e7e92a80f79cbfd1ef8070edfa926847825/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalRowCountCache.java#L90-L95)
 runs on `RowCountRefreshExecutor` and directly calls 
`table.fetchRowCountWithMetaCache(...)` without the owning catalog's 
`ExecutionAuthenticator`.
   - 
[`PaimonExternalTable.fetchRowCount`](https://github.com/apache/doris/blob/35854e7e92a80f79cbfd1ef8070edfa926847825/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java#L203-L214)
 executes its full table/scan path under that background caller context.
   - Paimon 1.1.1's `CachedClientPool` uses a JVM-static cache. Its default key 
is client class + HMS URI + the Hive identifier and excludes UGI/user name. 
Pool entries use `expireAfterAccess`, with Doris supplying a five-minute 
default.
   - New/expanded Paimon pool connections are created on the thread that needs 
them. The first connection is also forced eagerly by `CachedClientPool`'s 
constructor via `run(client -> null)`.
   
   ### Corrections / remaining evidence gap
   
   1. A Doris Paimon table-cache miss does not directly call HMS unwrapped. 
[`PaimonMetadataCache.loadTableCacheValue`](https://github.com/apache/doris/blob/35854e7e92a80f79cbfd1ef8070edfa926847825/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataCache.java#L85-L98)
 calls `PaimonExternalCatalog.getPaimonTable`, whose [`catalog.getTable(...)` 
call is 
wrapped`](https://github.com/apache/doris/blob/35854e7e92a80f79cbfd1ef8070edfa926847825/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java#L153-L175).
 Therefore, the missing outer wrapper is real, but the exact lazy HMS operation 
that escapes the nested wrapper still needs to be identified.
   
   2. The `ThriftHMSCachedClient` claim is not correct for this revision. There 
is only one `new ThriftHMSClient(...)` call site, and [`getClient()` already 
invokes it inside 
`ugiDoAs(...)`](https://github.com/apache/doris/blob/35854e7e92a80f79cbfd1ef8070edfa926847825/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/ThriftHMSCachedClient.java#L673-L681).
 Individual RPCs are wrapped as well. No additional constructor wrapping is 
indicated there.
   
   3. Paimon pool eviction is not a fixed five-minute cycle; it is expiration 
after five minutes without pool-cache access. Paimon's higher-level table 
caches can make such idle periods plausible even while queries continue.
   
   4. Adding `user_name` to Paimon's cache key is useful cross-catalog 
isolation, especially when catalogs share an HMS URI but use different users. 
It is defense-in-depth, not a substitute for executing the whole background 
operation under the correct UGI: the key is captured when `CachedClientPool` is 
constructed, while a pool recreated after eviction is still instantiated by the 
later calling thread.
   
   ### Information needed to close the causality gap
   
   - Redacted `SHOW CREATE CATALOG` output for both the Paimon and Hive 
catalogs, including `hadoop.username` and all Paimon/table/client-pool cache 
settings.
   - One complete timestamped log window covering pool eviction/cleanup, every 
`RetryingMetaStoreClient ... ugi=...` creation, the row-count-cache activity, 
and the first failed query. The logger/class or temporary creation-site stack 
instrumentation is needed to distinguish Paimon's `HiveClientPool` from Doris 
`ThriftHMSCachedClient`; the thread name alone cannot do that because both 
catalogs target the same HMS.
   - If reproducible in staging, temporarily shorten the relevant cache 
expirations and record `UserGroupInformation.getCurrentUser()` plus a stack at 
each HMS client creation. This should demonstrate exactly which operation opens 
the process-user connection.
   
   ### Recommended next steps
   
   1. Add the owning catalog's `ExecutionAuthenticator.execute(...)` around 
`fetchRowCountWithMetaCache(...)` in `ExternalRowCountCache.loadRowCount`. This 
is the correct task boundary and protects any current or future lazy external 
I/O.
   2. Add an FE unit test with a recording authenticator proving that both 
`fillMetaCache=true` and `false`, including the asynchronous loader, execute 
inside the catalog authentication context.
   3. Add a focused Paimon regression/integration test that forces pool 
eviction and verifies all replacement HMS connections retain the catalog user. 
Also test two catalogs sharing one HMS URI with different `hadoop.username` 
values.
   4. Do not change `ThriftHMSCachedClient` construction for this issue unless 
new evidence shows another creation path.
   
   `HADOOP_USER_NAME=hive` is a reasonable incident workaround, but it changes 
the process-wide fallback identity and is unsafe as a permanent solution where 
catalogs intentionally use different users.
   
   The reported symptom currently demonstrates a correctness/availability 
failure, not a Doris RBAC bypass. If testing shows a lower-privileged catalog 
or query can borrow a more-privileged UGI and read data it should not access, 
that should be escalated separately as security-sensitive.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to