Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add multi-part snapshot upload support #53

Merged
merged 8 commits into from Sep 25, 2018
Merged

Add multi-part snapshot upload support #53

merged 8 commits into from Sep 25, 2018

Conversation

swapnilgm
Copy link
Contributor

Signed-off-by: Swapnil Mhamane swapnil.mhamane@sap.com

What this PR does / why we need it:
This PR change the logic for uploading snapshots. Now, the snapshot from etcd is stored locally in temporary file. And then we will upload the snapshot in chunks to cloud provider's object store using multi-part upload functionality. In case one of the chunk upload fails then we will only retry upload of that chunk saving cost and time involved in whole upload as per old implementation.

Which issue(s) this PR fixes:
Fixes #48 #45

Special notes for your reviewer: NA

Release note:

Snapshot are now uploaded in chunks, considering only erroneous chunk upload in case of failure.

@swapnilgm swapnilgm added kind/enhancement Enhancement, improvement, extension reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies status/in-progress Issue is in progress/work size/s Size of pull request is small (see gardener-robot robot/bots/size.py) component/etcd-backup-restore ETCD Backup & Restore exp/intermediate Issue that requires some project experience platform/all priority/normal area/backup Backup related needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 21, 2018
@swapnilgm swapnilgm added size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies size/s Size of pull request is small (see gardener-robot robot/bots/size.py) status/in-progress Issue is in progress/work needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 3, 2018
Swapnil Mhamane added 6 commits September 3, 2018 16:54
Add multi-part snapshot upload support for Swift
026b5bb 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Add multi-part upload logic for aws
57bccd4 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Add composite object upload for GCS
9c0c1d5 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Add block uplaod support for ABS
d5a9302 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Update gitignore
8035a3a 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Refactor snapstore pkg
ea225c3 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 3, 2018
Modify test cases and mock client
d686a16 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
@swapnilgm swapnilgm added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 5, 2018
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 5, 2018
@swapnilgm swapnilgm changed the title Add multi-part snapshot upload support for Swift Add multi-part snapshot upload support Sep 10, 2018
georgekuruvillak
georgekuruvillak reviewed Sep 19, 2018
View reviewed changes
Copy link
Contributor

@georgekuruvillak georgekuruvillak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. I have some review comments. Do check it out.

pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/gcs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/s3_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/s3_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/snapshot.go Outdated Show resolved Hide resolved
pkg/snapstore/swift_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/s3_snapstore_test.go Show resolved Hide resolved
Gabage collect part uplaods
d483869 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
@CLAassistant
Copy link

CLAassistant commented Sep 21, 2018 •
edited

CLA assistant check
All committers have signed the CLA.

@swapnilgm swapnilgm added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 21, 2018
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 21, 2018
georgekuruvillak
georgekuruvillak approved these changes Sep 25, 2018
View reviewed changes
Copy link
Contributor

@georgekuruvillak georgekuruvillak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@georgekuruvillak georgekuruvillak merged commit f4c524d into gardener:master Sep 25, 2018
@swapnilgm swapnilgm deleted the multi-part-upload branch December 11, 2018 09:13
@PadmaB PadmaB added this to the 0.4.0 milestone Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgekuruvillak georgekuruvillak georgekuruvillak approved these changes

Assignees
No one assigned
Labels
area/backup Backup related component/etcd-backup-restore ETCD Backup & Restore exp/intermediate Issue that requires some project experience kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/all size/m Size of pull request is medium (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.

Etcd Snapshot upload timeout
5 participants
@swapnilgm @CLAassistant @georgekuruvillak @PadmaB @gardener-robot-ci-1