924060929 commented on PR #67978: URL: https://github.com/apache/doris/pull/67978#issuecomment-6053474067
Reviewed head: `b15f0343ebe6a2f50d20a9d8abb16c78bea70a74`. **[P2] Do not treat an older replayed backend epoch as proof that the dispatched process ended.** At [LanceIndexJobDispatcher.java:253–258](https://github.com/apache/doris/blob/b15f0343ebe6a2f50d20a9d8abb16c78bea70a74/fe/fe-core/src/main/java/org/apache/doris/datasource/lance/job/LanceIndexJobDispatcher.java#L253-L258), any epoch difference is converted into a durable `BE_PROCESS_EPOCH_GONE` proof and releases the possible-live slot. However, recovered backend metadata can be older than the job's dispatch epoch: 1. The durable backend epoch is 55. A successful heartbeat publishes epoch 77 to the in-memory `Backend`. 2. The dispatcher reads 77, journals `RUNNING` with epoch 77, and sends the request. 3. The FE crashes before that heartbeat is journaled. 4. The new master replays backend epoch 55 and job epoch 77. The master-transfer sweep makes the job `UNKNOWN` while retaining its slot. 5. This sweep sees `55 != 77`, journals the termination proof, and releases the slot, even though the actual BE process can still be 77. A subsequent heartbeat reporting 77 does not restore the released ownership. This ordering is possible because [HeartbeatMgr.java:158–177](https://github.com/apache/doris/blob/b15f0343ebe6a2f50d20a9d8abb16c78bea70a74/fe/fe-core/src/main/java/org/apache/doris/system/HeartbeatMgr.java#L158-L177) applies responses to backend memory before journaling the whole heartbeat package. The independent dispatcher can journal a job in that gap. Starting the heartbeat daemon before the dispatcher does not wait for a fresh heartbeat after recovery. The directly established defect in this PR is the false durable termination proof. The current BE stub executes no worker; once the worker consumes this contract, a still-running invocation can be omitted from the per-BE capacity count and new jobs can exceed the intended cap. The same-name fence and unresolved quota remain held, so this finding does not imply same-index redispatch or demonstrated query corruption/crash. Please retain ownership until a successful heartbeat observed in the current master leadership proves a different actual process incarnation. A replayed `isAlive` flag or epoch inequality alone is insufficient. Add a recovery test with durable backend epoch 55 and job epoch 77: no release before a fresh observation, continued ownership when the fresh heartbeat reports 77, and release only after a genuinely different current incarnation is observed. Cover the heartbeat-memory-before-journal crash window as well. This is distinct from the earlier capacity-count and missing-backend comments: those checks assume a different observed epoch proves process replacement; this recovery ordering shows that premise is not always valid. Recommendation: **request changes**. This is a source-level review; I did not run builds or tests. -- 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]
