1996fanrui commented on PR #24003: URL: https://github.com/apache/flink/pull/24003#issuecomment-1903758592
> LGTM 👍 Thanks for your efforts, @1996fanrui, and for keeping up with my nitty comments. I enjoyed the discussions in this PR. Thanks @XComp for the patient review, I definitely learned some skills from your comments. I enjoyed the discussions as well. > About the commits: We can reorganize them a bit, I feel. You could keep [6fe8037](https://github.com/apache/flink/commit/6fe8037b40d8458a5524adfb352d96925ced63bd) and [f422804](https://github.com/apache/flink/commit/f422804e918a8ea3537deeb1c4f4f1c9676e1a8a) as separate hotfix commits (using the `[hotfix]` prefix rather than the Jira issue) because they improve the code base and we would want to keep them even if we decide to revert [FLINK-33565](https://issues.apache.org/jira/browse/FLINK-33565) (for whatever reason) in the future. WDYT? The idea makes sense to me. 👍🏻 I update the commit message for the last commit, but I didn't updated it for the first commit due to some reasons: 1. The first commit changed the behavior - Before this PR, archiving exception when restarting task instead of immediately. - It means, when one task failure, we can see the exception history after flink restart this task. - The first commit archives exceptions into the exception history immediately when they occur, instead of archiving them when restarting. 2. I hope the first commit is fine, but I'm afraid we missed somethings , so it's not a simple refactor or hotfix. WDYT? -- 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]
