Re: Reviewing freeze map code

On Wed, Jun 8, 2016 at 4:01 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> If we want to address both page level and tuple level inconsistencies, I
> could see below possibility.
>
> 1. An API that returns setof records containing a block that have
> inconsistent vm bit, a block where visible page contains dead tuples and a
> block where vm bit indicates frozen, but page contains non-frozen tuples.
> Three separate block numbers are required in record to distinguish the
> problem with block.
>
> Signature of API will be something like:
> pg_check_visibility_blocks(regclass, corrupt_vm_blkno OUT bigint,
> corrupt_dead_blkno OUT bigint, corrupt_frozen_blkno OUT boolean) RETURNS
> SETOF record

I don't understand this, and I think we're making this too
complicated. The function that just returned non-frozen TIDs on
supposedly-frozen pages was simple. Now we're trying to redesign this
into a general-purpose integrity checker on the eve of beta2, and I
think that's a bad idea. We don't have time to figure that out, get
consensus on it, and do it well, and I don't want to be stuck
supporting something half-baked from now until eternity. Let's scale
back our goals here to something that can realistically be done well
in the time available.

Here's my proposal:

1. You already implemented a function to find non-frozen tuples on
supposedly all-frozen pages. Great.

2. Let's implement a second function to find dead tuples on supposedly
all-visible pages.

3. And then let's call it good.

If we start getting into the game of "well, that's not enough because
you can also check for X", that's an infinite treadmill. There will
always be more things we can check. But that's the project of
building an integrity checker, which while worthwhile, is out of scope
for 9.6.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company