You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by uce <gi...@git.apache.org> on 2016/08/12 14:56:35 UTC
[GitHub] flink pull request #2366: [FLINK-4322] Unify CheckpointCoordinator and Savep...
GitHub user uce opened a pull request:
https://github.com/apache/flink/pull/2366
[FLINK-4322] Unify CheckpointCoordinator and SavepointCoordinator
The CheckpointCoordinator now also takes over the role of the SavepointCoordinator. Savepoints are just like other checkpoints - they only store the metadata in addition. Restoring from a savepoint means loading it into the CheckpointStore at startup.
This simplifies the code quite a bit. We get rid of the savepoint coordinator and related classes and cumbersome restoring logic in the main code. For the tests, we can replace some integration tests with unit tests.
`PendingSavepoint` instances are finalized to become a `CompletedCheckpoint` like regular `PendingCheckpoint` instances, but in addition store the savepoint meta data and complete a Promise for callbacks. `PendingSavepoints` cannot be subsumed and a `CompletedCheckpoint` from a savepoint does not delete its associated state when being disposed.
@StephanEwen did most of the work and I added and fixed some tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/uce/flink savepointunify
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2366.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2366
----
commit 9d13d3b9c78b5fe6ec436c476492a82b846338aa
Author: Stephan Ewen <se...@apache.org>
Date: 2016-08-08T17:18:44Z
[FLINK-4322] [checkpointing] Unify CheckpointCoordinator and SavepointCoordinator
The CheckpointCoordinator now also takes over the role of the SavepointCoordinator.
Savepoints are just like other checkpoints - they only store the metadata in addition.
Restoring from a savepoint means loading it into the CheckpointStore at startup.
commit bcb6cf0b573314449437bc869febfc68f798b0f4
Author: Ufuk Celebi <uc...@apache.org>
Date: 2016-08-11T17:40:07Z
[FLINK-4322] [checkpointing] Add and fix tests
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2366
Yes. I can do it. Thanks.
Is it better to raise a JIRA for that for accounting or just raise a PR with the same issue ID?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2366
Reading the code once again, just few doubts/questions
->If a save point is triggered and that is happening with in the duration of the 'MINIMUM_TIME_BETWEEN_CHECKPOINTS' we still throw back a decline result? Is it not needed that since Save points are externally triggered we need to isolate that with the internal timing we maintain?
Correct me if am missing something here. Thanks.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2366
Looked very good. Fixed a minor issue (test log level) and merged this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2366
The unification looks great and now things are simple in terms of restoration. Thanks.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by ramkrish86 <gi...@git.apache.org>.
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/2366
Ya. So may be this code
`if (lastTriggeredCheckpoint + minPauseBetweenCheckpoints > timestamp) {`
The if block should also go inside the previous 'if' condition that checks for !savePoint, right?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2366
You are right, this was an oversight on my side. This check should also be within the `if (!isSavepoint())` block. Thanks for reviewing this.
Do you want to create a fix for that?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2366
Thanks for joining this effort. I'll try to have a look at the tests in the next days.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2366
JIRA issue never hurts. But in this case, it could piggy-bag on the previous issue. Your choice ;-)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2366: [FLINK-4322] Unify CheckpointCoordinator and SavepointCoo...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2366
Savepoints are excepted form the concurrent checkpoints and time between checkpoints limitation. These checks run only if the triggered checkpoint is not a savepoint (in the triggerCheckpoint(...)) method.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2366: [FLINK-4322] Unify CheckpointCoordinator and Savep...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2366
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---