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

crc checksum for header (key + value sizes) #30

Open
funny-falcon opened this issue Mar 9, 2020 · 1 comment
Open

crc checksum for header (key + value sizes) #30

funny-falcon opened this issue Mar 9, 2020 · 1 comment
Labels
wontfix This will not be worked on

Comments

@funny-falcon
Copy link

funny-falcon commented Mar 9, 2020

If data will be corrupted, then key+value size could be decoded into large value: unneccessary allocations and unneccessary disk read will follow.

It is better to add checsum for key+value size header to early detect such corruption.

And then crc32 checksum for data path could be resided in a header as well, therefore there will be no need to allocate buffer for both header and data.

I suppose, header could have following structure:

keySize = 2 bytes
typeAndValueSize = 4 bytes
dataCRC = 4 bytes
headerCRC = 4 bytes
Therefore headerCRC will check dataCRC as well.

@akrylysov
Copy link
Owner

It is better to add checsum for key+value size header to early detect such corruption.

Read/write performance and space amplification will take a hit - adding another checksum will increase the single key-value pair overhead.

If data will be corrupted, then key+value size could be decoded into large value: unneccessary allocations and unneccessary disk read will follow.

I'm not too worried about unnecessary reads and allocations - it may happen only on data corruption. The worst case scenario is Pogred allocates 4GB of memory and then discards the corrupted record - it won't prevent recovery.

I'm leaving the issue open, but I have no plans fixing it in the near future.

@akrylysov akrylysov added the wontfix This will not be worked on label Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants