Compress the snapshots(full as well as Incremental) before uploading to object store and decompress the snapshot before restoration. #293

ishan16696 · 2020-12-07T06:37:28Z

What this PR does / why we need it:
This PR added the compression/decompression feature. So, it compress/decompress the snapshots(full as well as Incremental) before/after uploading/downloading to/from object store.
It also supports the backward compatibility for restoring from non-compressed snapshots as well as when different compressionPolicy were used to take the snapshot.

Which issue(s) this PR fixes:
Fixes #255

Special notes for your reviewer:

Release note:

Add support for snapshot compression/decompression. Compression and compression policy can be configured through flags:  `--compress-snapshots` and `--compression-policy` respectively. Supported compression policies currently are `gzip` (default), `lzw` and `zlib`. Snapshot compression is disabled by default.

amshuman-kr

Thanks for the PR @ishan16696! It is looking good. Some small suggestions.

Also, could you please add some tests to cover the compression cases. Especially, for restoration in the mixed case (mixing of uncompressed and compressed backups with different algo)?

cmd/options.go

pkg/compressor/compressor.go

example/00-backup-restore-server-config.yaml

pkg/compressor/types.go

pkg/server/types.go

pkg/snapshot/snapshotter/snapshotter.go

pkg/snapshot/snapshotter/types.go

amshuman-kr

A small comment about the logging time. I think we can simply rely on existing time logging for download or upload which would be enough. No additional logging time for compression is required. We can still log that we are compressing/uncompressing.

pkg/compressor/compressor.go

pkg/snapshot/restorer/restorer.go

pkg/snapshot/snapshotter/snapshotter.go

amshuman-kr

Thanks for the changes. Once you have added the tests for the mixed case for restoration, this PR is good to be merged.

example/00-backup-restore-server-config.yaml

pkg/snapshot/restorer/restorer.go

shreyas-s-rao

@ishan16696 Thanks a lot for the well-written PR. Overall, code looks fine. I will test it out in the morning. Other than that, I have some minor code-related concerns/suggestions. Please address them. Thanks.
Also, please document your findings about compression and decompression (time taken, size reduction ratio, etc) for the different algorithms, so that it can serve as a reference point for someone new to compression and wanting to choose a suitable algorithm for running etcdbr.

pkg/compressor/init.go

pkg/compressor/types.go

pkg/snapshot/snapshotter/snapshotter.go

pkg/snapshot/restorer/restorer.go

pkg/snapshot/restorer/restorer_test.go

amshuman-kr

@ishan16696 Thanks a lot for the changes and also the tests. The PR is now looking good. Just some final comments below.

example/00-backup-restore-server-config.yaml

pkg/compressor/compressor.go

pkg/snapshot/restorer/restorer.go

pkg/snapshot/restorer/restorer_test.go

pkg/snapshot/snapshotter/snapshotter.go

amshuman-kr · 2021-01-19T05:55:39Z

Thanks @ishan16696 for the test case and the changes. I have only a few remaining questions.

In the restoration tests, it is not clear to me how the first runSnapshotter call reliably takes a full snapshot and also how we can be sure that the snapshot are completed by the time the function call returns.
The default vase for IsSnapshotCompressed for completeness.

ishan16696 · 2021-01-20T12:48:55Z

In the restoration tests, it is not clear to me how the first runSnapshotter call reliably takes a full snapshot

there is boolean variable here by which we can configure either to take fullSnapshot or deltaSnapshot.

how we can be sure that the snapshot are completed by the time the function call returns.

I am also not so sure .... but runSnapshotter is returning an error so by this error we can be sure that runSnapshotter is completed otherwise here test will get fail.

The default vase for IsSnapshotCompressed for completeness.

done

amshuman-kr

Thanks for the changes @ishan16696! LGTM

shreyas-s-rao

@ishan16696 Thanks for making all the changes. LGTM 👍🏼

gardener-robot added needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Dec 7, 2020

gardener-robot-ci-1 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 Dec 7, 2020

gardener-robot-ci-2 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 Dec 7, 2020

amshuman-kr suggested changes Dec 7, 2020

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Dec 7, 2020

gardener-robot-ci-1 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 Dec 8, 2020

gardener-robot-ci-3 removed 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 Dec 8, 2020

amshuman-kr suggested changes Dec 8, 2020

View reviewed changes

gardener-robot-ci-1 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 Dec 8, 2020

gardener-robot-ci-3 removed 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 Dec 8, 2020

gardener-robot-ci-2 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 reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 10, 2020

amshuman-kr reviewed Dec 10, 2020

View reviewed changes

example/00-backup-restore-server-config.yaml Outdated Show resolved Hide resolved

pkg/snapshot/restorer/restorer.go Outdated Show resolved Hide resolved

gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Dec 14, 2020

gardener-robot-ci-3 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 Dec 14, 2020

gardener-robot-ci-1 removed 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 Dec 14, 2020

gardener-robot-ci-2 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 Dec 14, 2020

gardener-robot-ci-1 removed 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 Dec 14, 2020

ishan16696 force-pushed the compression branch from e884b30 to fd0486e Compare December 14, 2020 08:11

gardener-robot-ci-2 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 Dec 14, 2020

gardener-robot-ci-3 removed 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 Dec 14, 2020

shreyas-s-rao requested changes Dec 14, 2020

View reviewed changes

gardener-robot-ci-1 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 reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 15, 2020

gardener-robot-ci-3 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 reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Dec 15, 2020

gardener-robot-ci-3 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 Dec 20, 2020

gardener-robot-ci-1 removed 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 Dec 20, 2020

gardener-robot-ci-2 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 Dec 23, 2020

gardener-robot-ci-1 removed 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 Dec 23, 2020

ishan16696 force-pushed the compression branch from 983d977 to 7fb4588 Compare December 24, 2020 05:05

gardener-robot-ci-1 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 Dec 24, 2020

gardener-robot-ci-2 removed 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 Dec 24, 2020

amshuman-kr suggested changes Jan 6, 2021

View reviewed changes

gardener-robot-ci-3 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 Jan 7, 2021

gardener-robot-ci-2 removed 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 Jan 7, 2021

gardener-robot 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 Jan 12, 2021

ishan16696 force-pushed the compression branch from 406989a to e27e10b Compare January 12, 2021 08:16

gardener-robot-ci-1 removed 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 Jan 12, 2021

ishan16696 force-pushed the compression branch from e27e10b to 5a972e8 Compare January 19, 2021 12:50

gardener-robot-ci-1 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 Jan 19, 2021

gardener-robot-ci-3 removed 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 Jan 19, 2021

Added support for compression/decompression of snapshots.

cd72e0f

ishan16696 force-pushed the compression branch from 5a972e8 to cd72e0f Compare January 19, 2021 16:26

gardener-robot-ci-3 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 reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 19, 2021

amshuman-kr approved these changes Jan 21, 2021

View reviewed changes

amshuman-kr mentioned this pull request Jan 21, 2021

Added support for backup compaction #301

Merged

shreyas-s-rao approved these changes Feb 2, 2021

View reviewed changes

shreyas-s-rao merged commit e6df91b into gardener:master Feb 2, 2021

This was referenced Feb 19, 2021

Integrate the etcdbr-compression feature from etcd-backup-restore with etcd-druid. gardener/etcd-druid#138

Merged

Update the helm charts for etcdbr-compression feature. #307

Merged

ishan16696 mentioned this pull request Mar 31, 2021

Introducing constants for snapshot compression policy gardener/etcd-druid#153

Merged

dependabot bot mentioned this pull request May 19, 2021

Bump github.com/gardener/gardener from 1.22.1 to 1.23.0 afritzler/garden-universe#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compress the snapshots(full as well as Incremental) before uploading to object store and decompress the snapshot before restoration. #293

Compress the snapshots(full as well as Incremental) before uploading to object store and decompress the snapshot before restoration. #293

ishan16696 commented Dec 7, 2020 •

edited by shreyas-s-rao

Loading

amshuman-kr left a comment

amshuman-kr left a comment •

edited

Loading

amshuman-kr left a comment

shreyas-s-rao left a comment

amshuman-kr left a comment

amshuman-kr commented Jan 19, 2021

ishan16696 commented Jan 20, 2021 •

edited

Loading

amshuman-kr left a comment

shreyas-s-rao left a comment

Compress the snapshots(full as well as Incremental) before uploading to object store and decompress the snapshot before restoration. #293

Compress the snapshots(full as well as Incremental) before uploading to object store and decompress the snapshot before restoration. #293

Conversation

ishan16696 commented Dec 7, 2020 • edited by shreyas-s-rao Loading

amshuman-kr left a comment

Choose a reason for hiding this comment

amshuman-kr left a comment • edited Loading

Choose a reason for hiding this comment

amshuman-kr left a comment

Choose a reason for hiding this comment

shreyas-s-rao left a comment

Choose a reason for hiding this comment

amshuman-kr left a comment

Choose a reason for hiding this comment

amshuman-kr commented Jan 19, 2021

ishan16696 commented Jan 20, 2021 • edited Loading

amshuman-kr left a comment

Choose a reason for hiding this comment

shreyas-s-rao left a comment

Choose a reason for hiding this comment

ishan16696 commented Dec 7, 2020 •

edited by shreyas-s-rao

Loading

amshuman-kr left a comment •

edited

Loading

ishan16696 commented Jan 20, 2021 •

edited

Loading