Re: Tricky bugs in concurrent index build

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Greg Stark <gsstark(at)mit(dot)edu> writes:
>> Hmmm. Or is that true. The problem may be somewhat easier since at least you
>> can be sure every tuple in the heap is in the index. So if you see a
>> DELETE_IN_PROGRESS either it *was* a constraint violation prior to the delete
>> and failing is reasonable or it's an update in which case maybe it's possible
>> to detect that they're part of the same chain?
>
> Unless we are willing to lock every single tuple while we insert it,
> this seems unfixable to me. Without a lock, the tuple could become
> DELETE_IN_PROGRESS immediately after we look at it.

I think there's some confusion here. This above paragraph was taken from some
thoughts about Hannu's suggestion of having a separate ALTER INDEX SET UNIQUE
command. That command might have an advantage over CREATE INDEX CONCURRENTLY
because it knows the index is already complete; it doesn't have to worry about
potential conflicts with tuples that it will only find later in the scan.

Effectively this is equivalent to making CREATE UNIQUE INDEX CONCURRENTLY
three phases. The first two phases would be a regular CREATE INDEX
CONCURRENTLY and the third phase would be what ALTER INDEX SET UNIQUE does
which is scan the index and verify that it's unique.

ALTER INDEX SET UNIQUE would have to perform a similar two-transaction dance
though. It would have to set the index unique, wait until everyone has seen
the new constraint. Then verify that the property is indeed unique, possibly
rolling back the constraint creation if it's not.

That would make the whole process of creating a unique index quite long. On
the plus side it would be a useful command in itself. Doing an index scan
might be pretty slow but if the table is mostly clean of dead and recently
dead tuples it won't have to visit the heap much and should still be much
quicker than building a new index. And it would itself be a concurrent
command.

> Actually it's worse than that. We could examine a tuple, see that
> it's good, include it in the uniqueness check. Then someone updates
> the tuple and puts the new version near the end of the table. By
> the time we reach that version, it could be committed good. There
> is absolutely no way that we could notice an issue without applying
> extremely expensive tests to *every* apparently-good tuple.

I think ALTER INDEX SET UNIQUE would not have this problem. It would only have
to look at tuples using its own snapshot and see if there's a violation. If
there isn't a violation as of its own snapshot then it can be sure later
transactions will preserve this property since the index was always complete
and it waited after creating the constraint.

> [ thinks for a bit... ] At least, it seems hopeless if we use
> SnapshotNow. Does it help if we use a real snapshot? I'm thinking
> pass 1 inserts exactly those tuples that are good according to a
> snap taken at its beginning, and then pass 2 considers only tuples
> that are good according to a snap taken at *its* beginning. But
> having consumed no caffeine yet this morning, I'm not sure I can
> spot any flaws that might exist in this idea.

What about tuples that are inserted and committed in the window between the
two phases. Ie, they're RECENTLY_DEAD but not in phase2's snapshot.

Or do you mean we use SatisfiesVacuum to determine what to insert but
SatisfiesSnapshot to determine whether to check uniqueness?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com