[GitHub] [flink] zentol commented on issue #8410: [FLINK-11159] Allow configuration whether to fall back to savepoints for restore

[GitBox <gi...@apache.org>]

zentol commented on issue #8410: [FLINK-11159] Allow configuration whether to fall back to savepoints for restore
URL: https://github.com/apache/flink/pull/8410#issuecomment-505172747
 
 
   The CompletedCheckpointStore is indeed quite sketchy; there are better ways to share logic ...
   
   The mocking is _less_ problematic in my view given that the test files we're modifying are already filled to the brim with mocking (and IIRC the checkpoint coordinator is notoriously difficult to interact with in tests without mocking).
   Naturally I'd prefer if we'd rework these tests entirely, but we can't just put this burden on the next guy to touch these tests. That said there are quite a few places though where we could mocking though this in simple ways. (e.g., there's no reason to mock a StreamStatehandle)
   
   It also seems like the documentation is rather light on potential downsides to using this feature, which were discussed in the JIRA.
   
   There's quite a bit of cleanup to be done here, as such I generally wouldn't oppose reverting it. However, there's a time constraint in play with the feature freeze coming up. Would we be fine for delaying this feature? If not, should we find a committer first to either review a cleaned up version, or even do the cleanup itself, before reverting it?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services