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.

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

Add code to api.RaftSnapshot to detect incomplete snapshots #12388

Merged
merged 3 commits into from Sep 7, 2021

Conversation

ncabatoff
Copy link
Contributor

An incomplete snapshot is one that doesn't include the final SHA256SUMS.sealed file.

@vercel vercel bot temporarily deployed to Preview – vault August 20, 2021 14:36 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook August 20, 2021 14:36 Inactive
@ncabatoff ncabatoff marked this pull request as ready for review August 20, 2021 15:23
@ncabatoff ncabatoff requested a review from a team August 20, 2021 18:21
@ncabatoff
Copy link
Contributor Author

To test whether or not my implementation is a memory hog, I added the following to the end of OperatorRaftSnapshotSaveCommand.Run in operator_raft_snapshot_save.go:

	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	c.UI.Info(fmt.Sprintf("totallocs=%d sys=%d", m.TotalAlloc, m.Sys))

I wrote 100k small keys to a raft cluster and took a snapshot with 1.8.1 and the above change:

totallocs=14887512 sys=76366856

With my branch and the above change:

totallocs=14945856 sys=75842568

(there's a bit of variability, but re-runs yielded similar numbers). The snapshot file was 11MB uncompressed, and I'm pretty sure if we held it all in memory those memory numbers would've gone up substantially. This confirms my understanding from code inspection of archive/tar, I just wanted to double check.

@ncabatoff
Copy link
Contributor Author

Fixes #12168

@ncabatoff ncabatoff requested a review from a team August 25, 2021 17:16
Copy link
Contributor

@raskchanky raskchanky 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, just a question about error handling

api/sys_raft.go Show resolved Hide resolved
Copy link
Contributor

@sgmiller sgmiller 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, love the injection of the error in the seal to trigger it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incomplete snapshots returned by snapshot save command
3 participants