Re: Tricky bugs in concurrent index build

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

> I think we can solve this by having IndexBuildHeapScan not index
> DELETE_IN_PROGRESS tuples if it's doing a concurrent build.

Sure

> It's entirely possible for a tuple that is RECENTLY_DEAD or
> DELETE_IN_PROGRESS to have no entry in the index, if it was inserted
> during the first pass, and then the deletion occurred after the first
> pass (and either has or hasn't committed yet).

Egads. That's nasty indeed.

> Furthermore, the patch also tries to insert RECENTLY_DEAD tuples, which is
> good for MVCC coverage, but wrong for uniqueness checking --- keep in mind
> that in the second pass, we are just doing normal index insertions, and so
> anything we insert into the index will be uniqueness-checked as though still
> alive. We could get a uniqueness failure that should not occur, eg from
> trying to insert the old version of a recently-updated row.

Hm, I hadn't absorbed the purpose of isAlive and the distinction between live
for uniqueness checks and live for index build purposes.

Is it not possible to brute force this adding an AM method to insert without
the uniqueness check? That would mean the index build would fail even if the
transaction eventually aborts though. (or even if it has already aborted?)

[ extended description of complex footwork involving more waiting while
holding locks ]

> Have I missed anything? This is tricky stuff.

Wow, that seems pretty unsatisfactory, all the waiting and locking sounds
awful. If you have a lot of update transactions starting continuously you
could keep bumping into this situation and repeatedly have to wait for new
transactions to end.

It also seems like a lot of code :(

> In any case it's clear that the patch still needs major work.
> Greg, do you have cycles to spare now?

I do. But I'll have to spend some time just rereading the code and your
comments to convince myself that all this waiting and locking is the best
solution.

--
greg