Doris-Breakwater commented on issue #66133: URL: https://github.com/apache/doris/issues/66133#issuecomment-5091863234
Breakwater-GitHub-Analysis-Slot: slot_bd27765b6413 This content is generated by AI for reference only. Initial assessment: this is a real correctness gap in the opt-in Hive SQL-result-cache path on current `master` (`e7b7f1d1359a46622dbdc98a493681674698e73e`), but the reproduction is missing one required setting. The issue currently has no labels; `area/sql/cache` and `area/extension/hive` look appropriate. The code path is: - Hive result caching is separately gated by `enable_hive_sql_cache`, which defaults to `false`. `BindRelation` records an `HMSExternalTable` in `SqlCacheContext` only when that flag is enabled ([code](https://github.com/apache/doris/blob/e7b7f1d1359a46622dbdc98a493681674698e73e/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java#L878-L893)). - `SqlCacheContext` snapshots `HMSExternalTable.getUpdateTime()` as the table version, and `NereidsSqlCacheManager.tablesOrDataChanged()` later validates only by comparing that value with the current in-memory value ([capture](https://github.com/apache/doris/blob/e7b7f1d1359a46622dbdc98a493681674698e73e/fe/fe-core/src/main/java/org/apache/doris/nereids/SqlCacheContext.java#L192-L213), [validation](https://github.com/apache/doris/blob/e7b7f1d1359a46622dbdc98a493681674698e73e/fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java#L431-L510)). - For HMS tables, initialization derives `updateTime` from `transient_lastDdlTime` (or the FE clock only when that parameter is absent); it does not derive a data version from the storage files ([code](https://github.com/apache/doris/blob/e7b7f1d1359a46622dbdc98a493681674698e73e/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalTable.java#L727-L745)). `CacheAnalyzer.buildCacheTableForHiveScanNode()` also uses the same table-level value. - A hit is resolved in `ConnectProcessor.parseFromSqlCache()` before normal analysis and scan split/file enumeration ([code](https://github.com/apache/doris/blob/e7b7f1d1359a46622dbdc98a493681674698e73e/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java#L416-L465)). Therefore, updating `HMSExternalTable.updateTime` only when files are observed by a normal scan would not close this bug: the stale-hit path never performs that scan. - The FE SQL-cache entry uses `expireAfterAccess`, not a fixed maximum age. With the default 300-second idle expiry, repeated hits can keep an otherwise stale entry alive indefinitely. So the reported mechanism is confirmed when both `enable_sql_cache=true` and `enable_hive_sql_cache=true`. It is not enabled for HMS tables by the default settings. An HMS DDL/event, a Doris Hive insert, or `REFRESH TABLE` changes the in-memory version; `REFRESH TABLE` also invalidates the Hive schema/partition/file metadata caches. A storage-only rewrite supplies none of those signals. Suggested immediate workaround: keep `enable_hive_sql_cache=false` for catalogs/tables whose files can be changed outside HMS, or make the external writer issue `REFRESH TABLE <catalog>.<db>.<table>` after the replacement and before readers are allowed to query. Suggested fix direction: 1. Do not fix this by merely assigning a max file modification time to `HMSExternalTable.updateTime` during split generation. Besides missing cache-hit queries, that value could come from the existing Hive file-list cache rather than a fresh listing. 2. Decide and document the correctness contract for `enable_hive_sql_cache`. The correctness-first short-term fix is to decline SQL-result caching for HMS tables when no authoritative change token is available. 3. If storage-only changes must remain cacheable, persist the exact scanned partition/file manifest in the SQL-cache context and revalidate it from storage before returning a cached result, bypassing stale file-list metadata. The current Hive `ScanTable` record does not retain the selected partitions/files. The fingerprint must cover at least path, length, and modification time, with storage-specific stronger identity (for example generation/version/ETag where reliable) if same-path replacement can preserve the other attributes. If a fresh identity cannot be obtained, fail closed with a cache miss. This has an unavoidable listing/stat cost and should be evaluated explicitly rather than hidden behind `updateTime`. 4. Add an external Hive regression covering: establish a `PhysicalSqlCache` hit with both flags enabled; replace files without changing HMS; verify no stale cache hit after the fix; and verify the existing HMS-event/`REFRESH TABLE` invalidation path. Include partitioned and non-partitioned tables if manifest validation is implemented. Information still needed from the reporter to confirm the exact observed deployment and distinguish SQL-result-cache staleness from the separate Hive file-list cache: - Doris version/commit and deployment mode. - Outputs of `SHOW VARIABLES LIKE 'enable_sql_cache'`, `SHOW VARIABLES LIKE 'enable_hive_sql_cache'`, `SHOW FRONTEND CONFIG LIKE 'cache_enable_sql_mode'`, and `SHOW FRONTEND CONFIG LIKE 'expire_sql_cache_in_fe_second'`. - Redacted `SHOW CREATE CATALOG`, table DDL, exact query, and whether `EXPLAIN PHYSICAL PLAN <query>` contains `PhysicalSqlCache` on the stale run. - Before/after file manifests (path, size, modification time, and object generation/ETag if available), plus the unchanged HMS `transient_lastDdlTime`. - Results of the same query with `enable_hive_sql_cache=false`, and after `REFRESH TABLE`. Those two checks isolate the SQL-result cache from cached Hive file metadata and storage-consistency behavior. -- 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]
