Re: Reviewing freeze map code

On Wed, Jun 8, 2016 at 6:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> 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,

This new API was to address Andres's concern of checking block level
inconsistency as we do in lazy_scan_heap. It returns set of inconsistent
blocks.

>
> 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.
>

Your proposal sounds good, will send an updated patch, if there are no
further concerns.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com