Thread: Enhanced rmgr desc routines test !has_image, not has_data
Recent commits that enhanced rmgr desc routines (commits 7d8219a4 and 1c453cfd) dealt with records that lack relevant block data (and so lack anything to give a more detailed summary of) by testing !DecodedBkpBlock.has_image -- that is the gating condition that determines if we want to (say) output a textual array representation of the page offset number from a given nbtree VACUUM WAL record. Strictly speaking, this isn't the correct gating condition to test. We should be testing the *presence* of the relevant block data instead. Why test an inexact proxy for the condition that we care about, when we can just as easily test the precise condition we care about instead? This isn't just a theoretical issue. Currently, we won't display detailed descriptions of block data whenever wal_consistency_checking happens to be in use. At least for those records with relevant block data available to summarize that also happen to have an FPI that the REDO routine isn't supposed to apply (i.e. an FPI that is included in the record purely so that verifyBackupPageConsistency can verify that the REDO routine produces a matching image). Attached patch fixes this bug. -- Peter Geoghegan
Attachment
On Tue, Apr 18, 2023 at 02:36:40PM -0700, Peter Geoghegan wrote: > This isn't just a theoretical issue. Currently, we won't display > detailed descriptions of block data whenever wal_consistency_checking > happens to be in use. At least for those records with relevant block > data available to summarize that also happen to have an FPI that the > REDO routine isn't supposed to apply (i.e. an FPI that is included in > the record purely so that verifyBackupPageConsistency can verify that > the REDO routine produces a matching image). Yeah, I agree that your suggestion is more useful for debugging when a record includes both a block image and some data associated to it. So, +1. -- Michael
Attachment
On Tue, Apr 18, 2023 at 11:10 PM Michael Paquier <michael@paquier.xyz> wrote: > Yeah, I agree that your suggestion is more useful for debugging when a > record includes both a block image and some data associated to it. > So, +1. Okay, pushed that fix just now. -- Peter Geoghegan