Re: Reviewing freeze map code

On Wed, Jun 8, 2016 at 6:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Jun 8, 2016 at 4:01 AM, Amit Kapila <amit.kapila16@gmail.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