On Mon, Mar 6, 2023 at 12:36 PM Robert Haas <robertmh...@gmail.com> wrote: > So it seems that we still don't have a patch where the > value of a variable called lp_valid corresponds to whether or not the > L.P. is valid.
Here's a worked-over version of this patch. Changes: - I got rid of the code that sets lp_valid in funny places and instead arranged to have check_tuple_visibility() pass up the information on the XID status. That's important, because we can't casually apply operations like TransactionIdIsCommitted() to XIDs that, for all we know, might not even be in the range covered by CLOG. In such cases, we should not perform any HOT chain validation because we can't do it sensibly; the new code accomplishes this, and also reduces the number of CLOG lookups as compared with your version. - I moved most of the HOT chain checks from the loop over the predecessor[] array to the loop over the successor[] array. It didn't seem to have any value to put them in the third loop; it forces us to expend extra code to distinguish between redirects and tuples, information that we already had in the second loop. The only check that seems to make sense to do in that last loop is the one for a HOT chain that starts with a HOT tuple, which can't be done any earlier. - I realized that your patch had a guard against setting the predecessor[] when it was set already only for tuples, not for redirects. That means if a redirect pointed into the middle of a HOT chain we might not report corruption appropriately. I fixed this and reworded the associated messages a bit. - Assorted cosmetic and comment changes. I think this is easier to follow and more nearly correct, but what do you (and others) think? -- Robert Haas EDB: http://www.enterprisedb.com
0001-Implement-HOT-chain-validation-in-verify_heapam.patch
Description: Binary data