shenzhu commented on pull request #18926: URL: https://github.com/apache/flink/pull/18926#issuecomment-1053765399
Hey @fapaul , Thanks so much for your review! Yeah I read the code in `loadAndValidateCheckpoint` and indeed this would break state compatibility. Sorry I'm new to this code base and may have some noob questions: **Q1. Breaking State Compatibility** If I understand it correctly, this change will break state compatibility in the following case: ``` CHAIN[Map(maxParallelism: -1) -> Filter(maxParallelism: 300) -> Print(maxParallelism: 01)] -> maxParallelism will be -1 ``` Then if user creates a savepoint, upgrades to a Flink version that includes this change, this application can't start from that savepoint because the max parallelism changes from -1 to 300. And there's [a unit test in CheckpointMetadataLoadingTest](https://github.com/apache/flink/blob/master/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointMetadataLoadingTest.java#L91) that covers such case. However, if we are going to change the max parallelism of chained operators, that [condition in loadAndValidateCheckpoint](https://github.com/apache/flink/blob/1ed9ea2815ec34365525f23a66edb27227b3da01/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/Checkpoints.java#L167) will break anyway right? Since we can't hold two max parallelism values for the same operator **Q2. Adding Tests to Cover Restore Behavior** For adding a test to show the restore behavior, do you think the [test case mentioned above](https://github.com/apache/flink/blob/master/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointMetadataLoadingTest.java#L91) is sufficient or we should add a test case to start from the beginning(in DataStream API when we declare map/filter operators)? ``` transformations -> StreamGraph -> JobGraph(with new maxParallelism for chained operators) -> ExecutionGraph -> Test the restore behavior of this generated ExecutionGraph ``` Thanks! -- 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]
