Re: [DOCS] HOT - correct claim about indexes not referencing old line pointers

On Mon, Oct 2, 2023 at 2:55 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Sat, Sep 30, 2023 at 1:05 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > > This is why I discovered it: it says "indexes do not reference their
> > > page item identifiers", which is manifestly not true when talking
> > > about the root item, and in fact would defeat the whole purpose of HOT
> > > (at least in a old-to-new chain like Postgres uses).
> >
> > Yeah, but...that's not what was intended. Obviously, the index hasn't
> > changed, and we expect index scans to continue to give correct
> > answers. So it is pretty strongly implied that it continues to point
> > to something valid.
>
> I took a look at this. I agree with James that the current wording is
> just plain wrong.
>
>      periodic vacuum operations.  (This is possible because indexes
>      do not reference their <link linkend="storage-page-layout">page
>      item identifiers</link>.)
>
> Here, the antecedent of "their" is "old versions of updated rows". It
> is slightly unclear whether we should interpret this to mean (a) the
> tuples together with the line pointers which point to them or (b) only
> the tuple data to which the line pointers point. If (a), then it's
> wrong because we can't actually get rid of the root line pointer;
> rather, we have to change it to a redirect. If (b), then it's wrong
> because heap_page_prune() removes dead tuples in this sense whether
> HOT is involved or not. I can see no interpretation under which this
> statement is true as written.
>
> I reviewed James's proposed alternative:
>
> +     periodic vacuum operations. However because indexes reference the old
> +     version's <link linkend="storage-page-layout">page item identifiers</link>
> +     the line pointer must remain in place. Such a line pointer has its
> +     <literal>LP_REDIRECT</literal> bit set and its offset updated to the
> +     <link linkend="storage-page-layout">page item identifiers</link> of
> +     the updated row.
>
> I don't think that's really right either. That's true for the root
> line pointer, but the phrasing seems to be referring to old versions
> generally, which would seem to include not only the root, for which
> this is correct, and also all subsequent now-dead row versions, for
> which it is wrong.
>
> Here is my attempt:
>
> When a row is updated multiple times, row versions other than the
> oldest and the newest can be completely removed during normal
> operation, including <command>SELECT</command>s, instead of requiring
> periodic vacuum operations. (Indexes always refer to the <link
> linkend="storage-page-layout">page item identifiers</link> of the
> original row version. The tuple data associated with that row version
> is removed, and its item identifier is converted to a redirect that
> points to the oldest version that may still be visible to some
> concurrent transaction. Intermediate row versions that are no longer
> visible to anyone are completely removed, and the associated page item
> identifiers are made available for reuse.)

Hi Robert,

Thanks for reviewing!

I like your changes. Reading through this several times, and noting
Peter's comments about pruning being more than just HOT, I'm thinking
that rather than a simple fixup for this one paragraph what we
actually want is to split out the concept of page pruning into its own
section of the storage docs. Attached is a patch that does that,
incorporating much of your language about LP_REDIRECT, along with
LP_DEAD so that readers know this affects more than just heap-only
tuple workloads.

Regards,
James