Add incremental backup-restore mechanism #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
June 4, 2018 17:25
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
added
needs/review
swapnilgm
added
the
reviewed/do-not-merge
swapnilgm
changed the title
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
25e97a2 to
1cdc1b5
Compare
swapnilgm
added
kind/enhancement
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
1cdc1b5 to
6129c45
Compare
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
6129c45 to
55cd889
Compare
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
2417d8a to
36145e2
Compare
swapnilgm
removed
the
reviewed/do-not-merge
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
swapnilgm
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
swapnilgm
force-pushed
the
feature/incremental-backup
branch
2 times, most recently
from
b6b1531 to
66e83eb
Compare
swapnilgm
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
66e83eb to
cce3f13
Compare
Signed-off-by: Swapnil Mhamane <[email protected]>
swapnilgm
force-pushed
the
feature/incremental-backup
branch
from
cce3f13 to
048f7d2
Compare
|
@georgekuruvillak Please review and test it thoroughly. |
Signed-off-by: Swapnil Mhamane <[email protected]>
georgekuruvillak
added
needs/ok-to-test
cmd/restore.go
Outdated
| @@ -113,3 +112,21 @@ func initialClusterFromName(name string) string { | |||
| } | |||
| return fmt.Sprintf("%s=http://localhost:2380", n) | |||
| } | |||
|
|
|||
| // getLatestFullSnapshotAndDeltaSnapList resturns the latest snapshot | |||
pkg/initializer/initializer.go
Outdated
| @@ -135,3 +133,21 @@ func removeContents(dir string) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| // getLatestFullSnapshotAndDeltaSnapList resturns the latest snapshot | |||
| func getLatestFullSnapshotAndDeltaSnapList(store snapstore.SnapStore) (*snapstore.Snapshot, snapstore.SnapList, error) { | |||
There was a problem hiding this comment.
Can we avoid the function duplication? The interface meets 'GetLatest' for snapstore, ryt?
| ctx = context.TODO() | ||
| ) | ||
|
|
||
| for _, e := range events { |
There was a problem hiding this comment.
Why do we commit on subsequent loop? Can we not fetch the ops in the same loop and commit at the end of the loop? Can we not expect events to be sequential? If that was not the case, we would lose some events because of irregular ordering, wont we?
There was a problem hiding this comment.
We do expect events to be sequential here. On etcd we can have transaction with multile put and delete operation. In that case, we get separate events with same key-value -modification-revision. So, if trace the logic here properly, you can see the we are collecting ops and committing it under same transaction before kv.ModRevision change. And last commit out of for-loop is just for commit the events collected in last loop iterations.
| return applyEventsToEtcd(client, events[newRevisionIndex:]) | ||
| } | ||
|
|
||
| // applyDeltaSnapshot applies thw events from delta snapshot to etcd |
| return nil | ||
| } | ||
|
|
||
| // applyFirstDeltaSnapshot applies thw events from first delta snapshot to etcd |
| return e, nil | ||
| } | ||
|
|
||
| // applyDeltaSnapshot applies thw events from time sorted list of delta snapshot to etcd sequentially |
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
|
Thank you George for reviewing it. I have addressed the changes in next PR on the way. As discussed, i I shall merge this branch now. |
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cloeses #2