sollhui commented on PR #66173: URL: https://github.com/apache/doris/pull/66173#issuecomment-5291842622
Thanks for the fix. I found two remaining concerns: 1. **[P1] CREATE-log replay can become terminal before later state records are applied.** `gsonPostProcess()` is also invoked by `RoutineLoadJob.read()` for `OP_CREATE_ROUTINE_LOAD_JOB`. If parsing/validation fails, the CREATE record is materialized as `CANCELLED`; a subsequent `OP_CHANGE_ROUTINE_LOAD_JOB` calls `updateState()`, but `checkStateTransform()` rejects every transition from a final state. Consequently, the authoritative later state—and in particular the durable cancellation reason added by this PR—can be dropped during journal replay. `replayCreateRoutineLoadJob()` also registers this synthetic final job as a transaction callback. Please distinguish image recovery from CREATE-log deserialization, or otherwise make later replay operations authoritative, and add a byte-level CREATE → CHANGE → REMOVE replay test. 2. **[P2] The generated reason can report the wrong failure time.** When a pre-existing final job already has an `endTimestamp` but lacks a persisted `cancelReason`, the message uses that historical stop/cancel timestamp as the time of the current FE restart deserialization failure. Please capture `failureTimestamp = System.currentTimeMillis()` for the message, while assigning it to `endTimestamp` only when the latter is unset. The current checks are green, but the added tests do not exercise the byte-level journal replay sequence above. -- 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]
