[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user uce commented on the issue: https://github.com/apache/flink/pull/3442 I had a quick chat with Stephan about this. @StefanRRichter has an idea how to properly implement this. Closing this PR and unassigning the issue. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3442 What's the status of this PR @uce @tillrohrmann @StefanRRichter @StephanEwen ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/3442 Are we gonna get this in for 1.3? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/3442 Very much ok with me, my vote on this was actually on keeping old versioned code duplicated and immutable. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user uce commented on the issue: https://github.com/apache/flink/pull/3442 I agree with the equals/hashCode question. It has been introduced for the various involved classes by different people and not in this PR, so I think it's best handled as part of a different issue. Would you mind opening an issue for it? Regarding code refactoring: I was also skeptical about the value of refactoring this. I'm totally fine with keeping the code duplicated. @StefanRRichter Is that OK with you as well? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3442 Another thought from the discussion with @StefanRRichter : You refactor a lot to not have duplicate code. While this is good in general, I am wondering if we should not actually duplicate the code here, because we want the `V1` serialization code and savepoint code to be "immutable", meaning it should not be affected by changes to the `V2` code. Having a copy makes sure no accidental changes are made to the `V1` code when modifying the `V2` code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3442 Looks good in general. One thing that I stumbled across a lot in recent works on the checkpoints / savepoints is that they all implement `equals` and `hashCode` and delegate to the task states and handles, etc. Do we need to define semantic equality there? It seems fragile to me, because the state handles by themselves can in general not really make a good claim about equality. The `FileStateHandle` for example fails to define equality depending on whether there is a trailing '/' on a directory path or not. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #3442: [FLINK-5778] [savepoints] Add savepoint serializer with r...
Github user uce commented on the issue: https://github.com/apache/flink/pull/3442 Thanks for your review Stefan! I addressed your comments, but only then realized that the restriction to relative file state handle is actually a problem for externalized checkpoints. :-( They possibly store their meta data somewhere else and not as part of the checkpoint files. For them it's OK to not be relocatable. Still, I agree with your comment that this should be more explicitly allowed or disallowed. I gues that we should explicitly distinguish between savepoints and externalized checkpoints for the meta data serialization. I hope that Stephan's refactorings of the checkpoint stream creation logic will help here. I'll leave this PR open with your suggested changes and adjust it after Stephan's PR. Then I can either piggyback on his changes or see whether I need to add the required distinction myself. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
