Re: Reviewing freeze map code

On 2016-06-08 10:04:56 +0530, Amit Kapila wrote:
> On Tue, Jun 7, 2016 at 11:01 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:>
> > I think if we go with the pg_check_visibility approach, we should also
> > copy the other consistency checks from vacuumlazy.c, given they can't
> > easily be triggered.
>
> Are you referring to checks that are done in lazy_scan_heap() for each
> block?

Yes.

> I think the meaning full checks in this context could be (a) page
> is marked as visible, but corresponding vm is not marked. (b) page is
> marked as all visible and has dead tuples. (c) vm bit indicates frozen, but
> page contains non-frozen tuples.

Yes.

> I think right now the design of pg_visibility is such that it returns the
> required information at page level to user by means of various functions
> like pg_visibility, pg_visibility_map, etc. If we want to add page level
> checks in this new routine as well, then we have to think what should be
> the output if such checks fails, shall we issue warning, shall we return
> information in some other way.

Right.

> Also, I think there will be some duplicity
> with the already provided information via other functions of this module.

Don't think that's a problem. One part of the functionality then is
returning the available information, the other is checking for problems
and only returning problematic blocks.

> > Wonder how we can report both block and tuple
> > level issues. Kinda inclined to report everything as a block level
> > issue?
> >
>
> The way currently this module provides information, it seems better to have
> separate API's for block and tuple level inconsistency. For block level, I
> think most of the information can be retrieved by existing API's and for
> tuple level, this new API can be used.

I personally think simplicity is more important than detail here; but
it's not that important. If this reports a problem, you can look into
the nitty gritty using existing functions.

Andres