CalvinKirs commented on PR #67726: URL: https://github.com/apache/doris/pull/67726#issuecomment-5629202618
## Local review results after rebase and fixes Reviewed commit: `de093b21cb45ad8e33b524542bc3eb0487dcd03e` Reviewed base: `60042611fea1b18576470a7e3c49e14cd11243a4` Review date: 2026-09-11 **Result: no outstanding confirmed findings — 0 Blocker, 0 Major, 0 Minor, 0 Nit.** The local review followed the `doris-repo-review` workflow with independent agents. Three earlier repair/review rounds found issues; the third round was not marked converged. After the final fix, a separate verification covered adapters (45 files), core/framework (38 files), and an independent downstream-path crosscheck. All 83 changed files have review coverage; unchanged files carried forward earlier review with content comparison. The final verification found no additional substantiated issue. ### Main fixes verified - Caffeine removal callbacks are deferred outside maintenance locks while preserving reservation synchronization and generation fencing. - Catalog property publication and cache retirement are coordinated; selective retirement still performs whole-catalog invalidation, including backend topology and row-count caches. - Unknown future configuration keys remain compatible, while consumed invalid settings are validated. Disabled caches do not perform weighted preparation. - Iceberg metadata ownership/accounting and rejected-entry read isolation were checked. In particular, [IcebergTableCache](https://github.com/apache/doris/blob/de093b21cb45ad8e33b524542bc3eb0487dcd03e/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergTableCache.java#L277-L280) now preserves successfully serialized metadata for an uncached borrower when estimation is incomplete; cache admission still rejects the entry. The strengthened test reproduces the original failure and verifies real SDK manifest-length resolution after the fix. - Existing estimator, invalidation, parser and lease mechanisms were reused; no optimizer, LiteralExpr or ListPartitionItem changes were added. ### Validation - `run-fe-ut.sh`: **402 tests across 25 classes passed; 0 failures, errors or skips**. - `build.sh --fe`: FE/plugin packaging and Checkstyle passed. - The final Iceberg rejection test was verified failing with the original production code and passing with the fix. ### Limits No BE/C++ build, fresh deployed-cluster regression or mixed-version execution was performed in this repair run. The focused Iceberg test exercises SDK manifest resolution using an in-memory input, not a complete external-storage scan. Memory estimates are approximate; cache budgets do not cap transient query memory or total FE heap. Supported BE-first compatibility was reviewed, not reverse-order upgrading. This is an AI-assisted local review summary, **not a CI status check or an Apache maintainer approval**, and does not guarantee the absence of all possible defects. -- 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]
