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

wal: add Verify function to perform corruption check on wal contents #10603

Merged
merged 2 commits into from
Apr 12, 2019
Merged

wal: add Verify function to perform corruption check on wal contents #10603

merged 2 commits into from
Apr 12, 2019

Conversation

shreyas-s-rao
Copy link
Contributor

Signed-off-by: Shreyas Rao shreyas.sriganesh.rao@sap.com

To allow verification of the WAL contents with minimal memory footprint. Added a Verify function to the wal package that performs a corruption check on the WAL contents and reports any errors. This serves to be a streamlined version of the ReadAll method on the WAL struct, as it allows for a quick validation of the WAL contents before starting etcd from an existing data directory without the overhead of returning the entire WAL data in-memory to the caller. Also refactored the openAtIndex function to allow the Verify function to reuse code for creating decoder.

Fixes #10497

This is a cherry-pick of two commits on master:
3d6862f
bb3eb8f

@shreyas-s-rao
wal: add Verify function to perform corruption check on wal contents
b8e5945 
Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>
@shreyas-s-rao shreyas-s-rao mentioned this pull request Apr 2, 2019
Merged
@shreyas-s-rao
Copy link
Contributor Author

@gyuho Any idea what the error /bin/bash: ./test: No such file or directory in the tests means?
Anything to be done from my side?

@gyuho
Copy link
Contributor

gyuho commented Apr 3, 2019

@hexfusion Can you help cherry-pick this?

@hexfusion
Copy link
Contributor

hexfusion commented Apr 3, 2019 via email •
edited

@shreyas-s-rao
Copy link
Contributor Author

@hexfusion Could you please take a look? Thanks

@hexfusion
Copy link
Contributor

I will get to this today.

@hexfusion
Copy link
Contributor

looking now thanks for your patience

hexfusion
hexfusion approved these changes Apr 11, 2019
View reviewed changes
Copy link
Contributor

@hexfusion hexfusion left a comment •
edited

Choose a reason for hiding this comment

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

lgtm, passed unit, integration_e2e and functional tests locally

@hexfusion
Copy link
Contributor

@shreyas-s-rao can you rerun tests we have fixed them in the branch.

git commit --amend --no-edit
git push -f

@hexfusion
Copy link
Contributor

will merge on greens

@shreyas-s-rao
wal: Add test for Verify
6f8aa33 
Signed-off-by: Shreyas Rao <shreyas.sriganesh.rao@sap.com>
@shreyas-s-rao
Copy link
Contributor Author

@hexfusion Tests have passed successfully. Semaphore build on amd64 seems to have failed from TestCtlV3AuthFromKeyPerm. I'm guessing it's an unrelated issue.

@hexfusion
Copy link
Contributor

TestCtlV3AuthFromKeyPerm Is a flake Semaphore passed on previous run. Merging.

@hexfusion hexfusion merged commit e206a8b into etcd-io:release-3.3 Apr 12, 2019
@shreyas-s-rao
Copy link
Contributor Author

@hexfusion Thanks for helping with the PR merge!

@jingyih
Copy link
Contributor

jingyih commented Apr 15, 2019

cc @wenjiaswe

@shreyas-s-rao
Copy link
Contributor Author

@gyuho Do we have a timeline for the v3.3.13 patch release? Are there any more fixes on etcd to be backported or is the patch already ready to be released?

@shreyas-s-rao
Copy link
Contributor Author

/ping @gyuho

@PadmaB
Copy link

PadmaB commented Apr 30, 2019

@gyuho any update on v3.3.13 patch release? Thanks!

@PadmaB
Copy link

PadmaB commented May 4, 2019

@gyuho Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hexfusion hexfusion hexfusion approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shreyas-s-rao @gyuho @hexfusion @jingyih @PadmaB