Re: HOT patch - version 14

On 8/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:

Not if someone else releases lock before committing.  Which I remind you
is a programming technique we use quite a lot with respect to the system
catalogs.  I'm not prepared to guarantee that there is no code path in
core (much less in contrib or third-party addons) that doesn't do it on
a user table; even less that no such path will ever appear in future.
As things stand it's a pretty harmless error --- the worst consequence
I know of is that a VACUUM FULL starting right then might bleat and
refuse to do shrinking.  What you propose is to turn the case into a
cause of silent index corruption.

A more robust solution would be to wait out the source transaction if
CREATE INDEX comes across an INSERT_IN_PROGRESS or DELETE_IN_PROGRESS
HOT tuple.  Or you could throw an error, since it's not really likely to
happen (except maybe in a system catalog REINDEX operation).  But the
way the patch approaches this at the moment is far too fragile IMHO.

So if we don't do HOT update on system tables, the current algorithm
should work fine because we should never find a HOT updated tuple
in the system table and the error-out mechanism should ensure
that we don't corrupt user tables.

So I guess what you are suggesting is to turn on HOT on system tables
and then wait-out any DELETING/INSETING transaction if we find such
a tuple during CREATE INDEX. I think this would work and since we are
talking about system tables, the wait-out business won't be harmful - I
remember there were objections when I suggested this as a general solution.

If we approach it this way, we might also be able to jettison some of
the other complexity such as idxcreatexid.

> IMHO the extra step in C.I.C simplifies the index build.

If you make the change suggested above, I think you don't need to do
things differently in C.I.C.

I am not sure if I follow you correctly here. The issue with CIC is that
it works with a snapshot. So during initial index build, we might index
a tuple which is in the middle of a broken HOT chain. In the second phase,
we just don't need to index the tuple at the head of the chain, but also
remove the previously inserted tuple, otherwise there would be two
references from the index to the same root heap tuple.

The additional step which does two things:

1) Create catalog entry - stops any new broken HOT chains being created
2) Wait-out existing transactions - makes sure that when the index is built,
we only index the last committed good tuple in the chain.

>>
>> Since CREATE INDEX works with ShareLock on the relation, we can
>> safely assume that we can't find any DELETE_IN_PROGRESS tuples except
>> those deleted by our own transaction. The only exception is system
>> relation, but we don't do HOT updates on system relation.

Same issue as above: this makes correctness utterly dependent on nobody
releasing locks early.

> OK.  So if I get you correctly, you are suggesting to acquire cleanup lock.
> If we don't get that, we don't to any maintenance work. Otherwise, we prune
> and repair fragmentation in one go.

Yeah, that's what I'm thinking; then there's no need to track a separate
page state where we've pruned but not defragmented.