Pages checksum verification #736
Conversation
usernamedt
force-pushed
the
page-checksum-verification
branch
from
e39e8df
to
e694378
Compare
usernamedt
force-pushed
the
page-checksum-verification
branch
from
e694378
to
4183043
Compare
| } | ||
| fileReader, cfi.header.Size, err = ReadIncrementalFile(cfi.path, cfi.fileInfo.Size(), *p.incrementFromLsn, bitmap) |
There was a problem hiding this comment.
I've renamed fileReader to fileReadCloser because actually it is ReadCloser, not only reader
usernamedt
force-pushed
the
page-checksum-verification
branch
from
4183043
to
1dc6796
Compare
internal/backup_file_description.go
Outdated
|
|
||
| type BackupFileDescription struct { | ||
| IsIncremented bool // should never be both incremented and Skipped | ||
| IsSkipped bool | ||
| MTime time.Time | ||
| CorruptBlocks []uint32 `json:",omitempty"` |
There was a problem hiding this comment.
Add the actual number of corrupted blocks. Sometimes it helps to distinguish different bugs.
There was a problem hiding this comment.
And maybe a flag to keep all blocks.
| func (m *MultiCloser) Close() error { | ||
| var err error | ||
| for _, c := range m.closers { | ||
| // still call Close on each, even if one returns an error |
There was a problem hiding this comment.
hm, it will be better to save all error messages. You can combine it to a new error
| "unsafe" | ||
| ) | ||
|
|
||
| // This code is an adaptation of Postgres data page checksum calculation code written in Go. |
There was a problem hiding this comment.
I will trust that it has the same code.
| } | ||
|
|
||
| // VerifyPagedFileIncrement verifies pages of an increment | ||
| func VerifyPagedFileIncrement(path string, fileInfo os.FileInfo, increment io.Reader) ([]uint32, error) { |
usernamedt
force-pushed
the
page-checksum-verification
branch
from
599523f
to
50e27bd
Compare
|
@usernamedt awesome features! discovered by accident. Couldn't find anything in README or Postgres-related .md doc file. Could you please add info there? (I can help, but it would be good to have at least a draft) |
Hi, thanks for noting this, seems like I forgot to add documentation for this feature :) I'll fix this soon. As for now, I have a more or less detailed description here. |
In this PR I propose the page checksums verification functionality for
backup-pushI've adapted the Postgres page hash calculation (modified version of FNV-1a hash algorithm) from C origin:
https://github.com/postgres/postgres/blob/REL_12_STABLE/src/include/storage/checksum_impl.h
Also, I've also partially borrowed the logic of is_page_corrupted() from pg_page_verification tool: https://github.com/google/pg_page_verification/blob/master/pg_page_verification.c
Users can enable the page verification using
--verifyflag or by settingWALG_VERIFY_PAGE_CHECKSUMSenv variable. If found any, corrupted block numbers (currently no more than 10 of them) will be recorded to the backup sentinel json, for example: