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

Fix VerifyValueChecksum checks #1138

Merged
merged 1 commit into from
Dec 6, 2019
Merged

Conversation

tuesmiddt
Copy link
Contributor

@tuesmiddt tuesmiddt commented Nov 27, 2019

Fixes #1127

Issue was that crc32 value read from log was included when recomputing hash value for verification.


This change is Reviewable

@coveralls
Copy link

coveralls commented Nov 27, 2019

Coverage Status

Coverage increased (+0.05%) to 77.637% when pulling b9bd4b7 on tuesmiddt:master into 407e5bd on dgraph-io:master.

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this @tuesmiddt. The PR looks good to me 👍 💯

@jarifibrahim jarifibrahim merged commit e28642f into dgraph-io:master Dec 6, 2019
jarifibrahim pushed a commit that referenced this pull request Dec 6, 2019
PR #1138 fixed the checksum
issue but it was missing a test. This PR adds a test for it.
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
Fixes #1127 

The issue was that the crc32 value read from log was included when
recomputing hash value for verification.

(cherry picked from commit e28642f)
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
PR #1138 fixed the checksum
issue but it was missing a test. This PR adds a test for it.

(cherry picked from commit 1ee50f7)
manishrjain pushed a commit to outcaste-io/outserv that referenced this pull request Jul 6, 2022
PR dgraph-io/badger#1138 fixed the checksum
issue but it was missing a test. This PR adds a test for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

VerifyValueChecksum functionality is broken
3 participants