On 11/30/20 6:38 PM, David Steele wrote:
On 11/30/20 9:27 AM, Stephen Frost wrote:
* Michael Paquier (mich...@paquier.xyz) wrote:
On Fri, Nov 27, 2020 at 11:15:27AM -0500, Stephen Frost wrote:
* Magnus Hagander (mag...@hagander.net) wrote:
On Thu, Nov 26, 2020 at 8:42 AM Michael Paquier
<mich...@paquier.xyz> wrote:
But here the checksum is broken, so while the offset is something we
can rely on how do you make sure that the LSN is fine? A broken
checksum could perfectly mean that the LSN is actually *not* fine if
the page header got corrupted.
Of course that could be the case, but it gets to be a smaller and
smaller chance by checking that the LSN read falls within reasonable
bounds.
FWIW, I find that scary.
There's ultimately different levels of 'scary' and the risk here that
something is actually wrong following these checks strikes me as being
on the same order as random bits being flipped in the page and still
getting a valid checksum (which is entirely possible with our current
checksum...), or maybe even less.
I would say a lot less. First you'd need to corrupt one of the eight
bytes that make up the LSN (pretty likely since corruption will probably
affect the entire block) and then it would need to be updated to a value
that falls within the current backup range, a 1 in 16 million chance if
a terabyte of WAL is generated during the backup. Plus, the corruption
needs to happen during the backup since we are going to check for that,
and the corrupted LSN needs to be ascending, and the LSN originally read
needs to be within the backup range (another 1 in 16 million chance)
since pages written before the start backup checkpoint should not be torn.
So as far as I can see there are more likely to be false negatives from
the checksum itself.
It would also be easy to add a few rounds of checks, i.e. test if the
LSN ascends but stays in the backup LSN range N times.
Honestly, I'm much more worried about corruption zeroing the entire
page. I don't know how likely that is, but I know none of our proposed
solutions would catch it.
Andres, since you brought this issue up originally perhaps you'd like to
weigh in?
I had another look at this patch and though I think my suggestions above
would improve the patch, I have no objections to going forward as is (if
that is the consensus) since this seems an improvement over what we have
now.
It comes down to whether you prefer false negatives or false positives.
With the LSN checking Stephen and I advocate it is theoretically
possible to have a false negative but the chances of the LSN ascending N
times but staying within the backup LSN range due to corruption seems to
be approaching zero.
I think Michael's method is unlikely to throw false positives, but it
seems at least possible that a block would be hot enough to be appear
torn N times in a row. Torn pages themselves are really easy to reproduce.
If we do go forward with this method I would likely propose the
LSN-based approach as a future improvement.
Regards,
--
-David
da...@pgmasters.net