Ensuring the integrity of full snapshot before uploading it to the object store. #779

ishan16696 · 2024-09-23T07:25:52Z

What this PR does / why we need it:
It has been observed that while doing the restoration from full snapshot sometimes backup-restore failed to restore the etcd due to sometimes full snapshots got corrupted or missing a hash.
This PR is try to minimise the occurrence of such scenarios by verifying the integrity of full snapshot before uploading it to the object store.

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

Special notes for your reviewer:
This is how it's done:

Release note:

Ensure the integrity of the full snapshot prior to uploading it to the object store, thereby reducing the potential restoration failures.

ishan16696 · 2024-09-23T07:36:39Z

Performance tests have been performed with etcd of 7GB database size:

~ > etcdctl endpoint status --endpoints=http://127.0.0.1:2379 --cluster -w table
+-----------------------+------------------+---------+---------+-----------+------------+-----------+------------+--------------------+--------+
|       ENDPOINT        |        ID        | VERSION | DB SIZE | IS LEADER | IS LEARNER | RAFT TERM | RAFT INDEX | RAFT APPLIED INDEX | ERRORS |
+-----------------------+------------------+---------+---------+-----------+------------+-----------+------------+--------------------+--------+
| http://localhost:2379 | 8e9e05c52164694d |   3.5.9 |  7.0 GB |      true |      false |         3 |      35952 |              35952 |        |
+-----------------------+------------------+---------+---------+-----------+------------+-----------+------------+--------------------+--------+

Backups taken by backup-restore with their sizes on disk.

25M	Full-00000000-02000001-1726772100.gz
79M	Full-00000000-05532798-1726864342.gz
140M	Full-00000000-09083821-1726946137.gz
185M	Full-00000000-11680572-1727038676.gz
200K	Incr-11680573-11691572-1727038777.gz
200K	Incr-11691573-11702572-1727038778.gz
200K	Incr-11702573-11712987-1727038807.gz
68K	Incr-11712988-11716517-1727068649.gz

Then a big full-snapshot is triggered by backup-restore:

INFO[0536] Taking scheduled full snapshot for time: 2024-09-23 11:00:00.014743 +0530 IST  actor=snapshotter
{"level":"info","ts":"2024-09-23T11:00:00.038+0530","caller":"clientv3/maintenance.go:200","msg":"opened snapshot stream; downloading"}
INFO[0536] Total time taken by Snapshot API: 0.002619 seconds.  actor=snapshotter
INFO[0536] checking the full snapshot integrity with the help of SHA256  actor=snapshotter
{"level":"info","ts":"2024-09-23T11:00:06.902+0530","caller":"clientv3/maintenance.go:208","msg":"completed snapshot read; closing"}
INFO[0547] full snapshot SHA256 hash has been successfully verified.  actor=snapshotter
INFO[0547] start compressing the snapshot using gzip Compression Policy  actor=compressor
INFO[0547] Total time taken in full snapshot compression: 0.001206 seconds.  actor=snapshotter
INFO[0547] Successfully opened snapshot reader on etcd   actor=snapshotter
INFO[0578] Total written bytes: 6982844448               actor=compressor
INFO[0578] Total time to save full snapshot: 41.202852 seconds.  actor=snapshotter
INFO[0578] Successfully saved full snapshot at: Full-00000000-11716517-1727069410.gz  actor=snapshotter
INFO[0578] Applied watch on etcd from revision: 11716518  actor=snapshotter

Then restoration is triggered:

INFO[0007] Received start initialization request.        actor=backup-restore-server
INFO[0007] Updating status from New to Progress          actor=backup-restore-server
INFO[0007] Setting status to : 503                       actor=backup-restore-server
INFO[0007] Validation failBelowRevision:                 actor=backup-restore-server
INFO[0007] Validation mode: full                         actor=backup-restore-server
INFO[0007] Checking for data directory structure validity...
INFO[0007] Data directory structure invalid.
INFO[0007] Finding latest set of snapshot to recover from...
INFO[0007] Removing directory(default.etcd.part).
INFO[0007] Restoring from base snapshot: Full-00000000-11716517-1727069410.gz  actor=restorer
INFO[0007] start decompressing the snapshot with gzip compressionPolicy  actor=de-compressor
INFO[0009] Responding to status request with: Progress   actor=backup-restore-server
{"level":"warn","ts":"2024-09-23T13:04:23.060+0530","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"passthrough:///http://localhost:2379","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest balancer error: connection error: desc = \"transport: Error while dialing dial tcp [::1]:2379: connect: connection refused\""}
ERRO[0010] failed to get status of etcd endPoint: http://localhost:2379 with error: context deadline exceeded
INFO[0012] successfully fetched data of base snapshot in 4.938676375 seconds [CompressionPolicy:gzip]  actor=restorer
{"level":"warn","ts":"2024-09-23T13:04:27.060+0530","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"passthrough:///http://localhost:2379","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest balancer error: connection error: desc = \"transport: Error while dialing dial tcp [::1]:2379: connect: connection refused\""}
ERRO[0014] failed to get status of etcd endPoint: http://localhost:2379 with error: context deadline exceeded
INFO[0016] Responding to status request with: Progress   actor=backup-restore-server
{"level":"warn","ts":"2024-09-23T13:04:31.061+0530","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"passthrough:///http://localhost:2379","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest balancer error: connection error: desc = \"transport: Error while dialing dial tcp [::1]:2379: connect: connection refused\""}
ERRO[0018] failed to get status of etcd endPoint: http://localhost:2379 with error: context deadline exceeded
{"level":"info","ts":1727076872.963309,"caller":"membership/cluster.go:392","msg":"added member","cluster-id":"cdf818194e3a8c32","local-member-id":"0","added-peer-id":"8e9e05c52164694d","added-peer-peer-urls":["http://localhost:2380"]}
INFO[0020] No delta snapshots present over base snapshot.  actor=restorer
INFO[0020] Removing directory(default.etcd).
INFO[0020] Successfully restored the etcd data directory.
INFO[0020] Successfully initialized data directory for etcd.  actor=backup-restore-server

Full snapshot is successfully taken with integrity check. ✅
Successfully restored the etcd from the full snapshot. ✅
Snapshot-Compaction is also working as expected. ✅

anveshreddy18 · 2024-09-23T09:00:23Z

/assign

anveshreddy18

Thanks for the PR @ishan16696.

PR looks good, I just have 2 small suggestions. PTAL

pkg/etcdutil/etcdutil.go

anveshreddy18

I tested it and things are working good.
LGTM

seshachalam-yv · 2024-10-01T02:16:59Z

/assign

renormalize

Thanks for this much needed PR @ishan16696!

Just a few comments from my side.

Thanks.

pkg/compactor/compactor.go

cmd/compact.go

pkg/etcdutil/etcdutil.go

renormalize

Another suggestion for better error handling, as discussed.

pkg/etcdutil/etcdutil.go

renormalize

Tiny nit, but otherwise the PR looks in great condition.
Thanks.

pkg/etcdutil/etcdutil.go

renormalize

Thanks @ishan16696 for filling the gaps in my understanding in validation, and addressing all review comments!

renormalize · 2024-10-18T08:32:55Z

@ishan16696 could you rebase your PR on master? This will fix the failing integration tests.
Thanks.

…ct store.

Fix unit tests.

Rename few variables.

example/01-etcd-config.yaml

seshachalam-yv · 2025-01-28T10:49:34Z

pkg/etcdutil/etcdutil.go

+		return nil, err
+	}
+
+	if _, err := io.Copy(db, snapshotData); err != nil {


Performance Improvement:

io.Copy uses a default 32KB buffer, while io.CopyBuffer with hashBufferSize (4MB) reduces system calls significantly.

For example, copying a 1GB snapshot:

With io.Copy: ~32,768 system calls (32KB buffer)

With io.CopyBuffer: ~256 system calls (4MB buffer)

that's a good point but I'm not sure will it be significant improvement or not as etcd usually run on powerful VMs and using small buffer (32KB in io.copy) or big buffer (2MB or 4MB in io. CopyBuffer) has it's pros and cons:

small buffer will use less memory, and more no. of sys calls.

big buffer will use more memory and less no. of sys calls.

what about buffer of size 1MB for both which will reduce the sys calls as well as reduce the memory usage as well ? and we will use your benchmark test to analyse both case, wdyt ?

ishan16696 requested a review from a team as a code owner September 23, 2024 07:25

gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Sep 23, 2024

gardener-robot assigned anveshreddy18 Sep 23, 2024

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 Sep 24, 2024

anveshreddy18 reviewed Sep 25, 2024

View reviewed changes

pkg/etcdutil/etcdutil.go Outdated Show resolved Hide resolved

pkg/etcdutil/etcdutil.go Outdated Show resolved Hide resolved

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 Sep 25, 2024

anveshreddy18 approved these changes Sep 26, 2024

View reviewed changes

renormalize self-assigned this Sep 30, 2024

gardener-robot assigned seshachalam-yv Oct 1, 2024

shreyas-s-rao added this to the v0.31.0 milestone Oct 8, 2024

anveshreddy18 removed their assignment Oct 8, 2024

renormalize requested changes Oct 17, 2024

View reviewed changes

pkg/etcdutil/etcdutil.go Outdated Show resolved Hide resolved

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 Oct 17, 2024

renormalize reviewed Oct 18, 2024

View reviewed changes

pkg/etcdutil/etcdutil.go Outdated Show resolved Hide resolved

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 Oct 18, 2024

renormalize approved these changes Oct 18, 2024

View reviewed changes

renormalize removed their assignment Oct 18, 2024

shreyas-s-rao unassigned shreyas-s-rao and seshachalam-yv Dec 31, 2024

renormalize linked an issue Jan 8, 2025 that may be closed by this pull request

[Enhancement] Restoration failed due to full-snapshot taken by backup-restore is found to be corrupted or not having a hash #778

Open

shreyas-s-rao modified the milestones: v0.33.0, v0.34.0 Jan 21, 2025

ishan16696 added 8 commits January 22, 2025 14:59

Verify the integrity of full snapshot before uploading it to the obje…

fc2547e

…ct store.

Fix compact sub-command while checking integrity of full-snapshot

fae13bd

Fix unit tests.

Address review comments.

f1f0787

Address review comments.

77a4595

Address review comments 2.

e52e0d9

Address review comments 3.

3a12fc1

Address review comments 4.

0fa0b3f

Add few logs to debug.

2244885

Rename few variables.

ishan16696 force-pushed the verifyFullSnapshot branch from 9b55def to 2244885 Compare January 22, 2025 09:29

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 22, 2025

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 22, 2025

Address review comments.

bf64ddb

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 25, 2025

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 25, 2025

Refactoring the code.

3cf6bd2

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 Jan 27, 2025

shreyas-s-rao assigned shreyas-s-rao and seshachalam-yv and unassigned ishan16696 Jan 27, 2025

Reverted one change.

f866722

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 27, 2025

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 27, 2025

ishan16696 self-assigned this Jan 27, 2025

seshachalam-yv requested changes Jan 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensuring the integrity of full snapshot before uploading it to the object store. #779

Ensuring the integrity of full snapshot before uploading it to the object store. #779

ishan16696 commented Sep 23, 2024 •

edited

Loading

ishan16696 commented Sep 23, 2024 •

edited

Loading

anveshreddy18 commented Sep 23, 2024

anveshreddy18 left a comment

anveshreddy18 left a comment

seshachalam-yv commented Oct 1, 2024

renormalize left a comment

renormalize left a comment

renormalize left a comment

renormalize left a comment

renormalize commented Oct 18, 2024

seshachalam-yv Jan 28, 2025

ishan16696 Jan 29, 2025 •

edited

Loading

Ensuring the integrity of full snapshot before uploading it to the object store. #779

Are you sure you want to change the base?

Ensuring the integrity of full snapshot before uploading it to the object store. #779

Conversation

ishan16696 commented Sep 23, 2024 • edited Loading

ishan16696 commented Sep 23, 2024 • edited Loading

anveshreddy18 commented Sep 23, 2024

anveshreddy18 left a comment

Choose a reason for hiding this comment

anveshreddy18 left a comment

Choose a reason for hiding this comment

seshachalam-yv commented Oct 1, 2024

renormalize left a comment

Choose a reason for hiding this comment

renormalize left a comment

Choose a reason for hiding this comment

renormalize left a comment

Choose a reason for hiding this comment

renormalize left a comment

Choose a reason for hiding this comment

renormalize commented Oct 18, 2024

seshachalam-yv Jan 28, 2025

Choose a reason for hiding this comment

ishan16696 Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

ishan16696 commented Sep 23, 2024 •

edited

Loading

ishan16696 commented Sep 23, 2024 •

edited

Loading

ishan16696 Jan 29, 2025 •

edited

Loading