Baymine commented on PR #66312: URL: https://github.com/apache/doris/pull/66312#issuecomment-5161532590
Thanks for the thorough review — all seven points were valid and are addressed in d23ac54a95721b2379770c83232b516869636bcb. Highlights: - **P1 (disk cleanup):** `isProfileLoaded` now means only "the on-disk index is complete" and is set solely on the success path. A failed/partial load records a separate `isProfileLoadFailed` flag that stops loader respawn **without** enabling `deleteBrokenProfiles()`/`deleteOutdatedProfilesFromStorage()`, so valid stored profiles can no longer be deleted from a partial in-memory index. - **Recheck after CAS:** stale pre-CAS reads can no longer start a redundant second cold load. - **Launch failure:** `Thread.start()` failure now releases loader ownership and records a terminal failure instead of stranding it. - **Tests:** deterministic single-loader assertion scoped to the tested manager; sync caller run on its own thread and asserted to block then complete; failure test now reaches the injected `pushProfile` failure via a valid profile id and asserts `isProfileLoaded` stays false; teardown waits on the real loader's terminal state. Verified locally: `fe-core` compiles and `ProfileManagerTest` passes (31 tests, 0 failures/errors). run buildall -- 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]
