asdf2014 commented on PR #65788: URL: https://github.com/apache/doris/pull/65788#issuecomment-5016219052
The follow-up commit addresses every blocker in this review. Summary of what changed and why: 1. Timeout/ownership (the detached fanout): the per-tablet decision syncs no longer run on raw detached bthreads. They run on a dedicated, bounded, engine-owned thread pool (`CloudStorageEngine::query_cache_delta_sync_pool`), so there is a fixed width and a bounded queue instead of an unbounded per-query fanout, `stop()` drains it before `_meta_mgr`/`_tablet_mgr` are destroyed so no task can outlive the engine, and a shared `abandoned` flag lets a task that outran the wait skip its RPC so a meta-service brownout drains fast instead of piling up. 2. Cloud MOW write-back / raw-option gating: BE no longer keys write-back on the raw prefer knob for cloud MOW. FE now reports the selected index's MOW state through a new optional thrift field and BE keeps write-back for the version-exact MOW read, so the exact path is no longer disabled on the one path where the knob does not apply. 3. Mixed-version: the new thrift field is optional and appended, so old-FE to new-BE defaults to the conservative (suppress) behavior and new-FE to old-BE ignores it. That is the BE-side default the review asked for. 4. Regression determinism: the routing-dependent metric assertions are gated to the shared-nothing path; on cloud the suite proves correctness by result consistency, and the cloud INCREMENTAL path itself is proven by deterministic BE unit tests. 5. Both clang-format checks are green now. Details are in the replies on each review thread. Two deliberate trade-offs, stated plainly: (1) the pre-sync tasks do not attach a per-query memory tracker, consistent with the sibling `CloudBackendService::sync_load_for_tablets` path and documented at the call site; (2) the cloud regression relies on result consistency plus the deterministic BE unit tests instead of the routing-dependent metric assertions. A follow-up could add a profile-based cloud assertion (checking `HitCacheStale` in the serving query's profile) if a topology-independent end-to-end signal is wanted. -- 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]
