RE: Avoiding deadlock errors in CREATE INDEX CONCURRENTLY

Hi Dhruv,

On Sun, June 30, 2019 at 7:30 AM, Goel, Dhruv wrote:
> > On Jun 10, 2019, at 1:20 PM, Goel, Dhruv <goeldhru@amazon.com> wrote:
> >> On Jun 9, 2019, at 5:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Andres Freund <andres@anarazel.de> writes:
> >>> On June 9, 2019 8:36:37 AM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>>> I think you are mistaken that doing transactional updates in
> >>>> pg_index is OK.  If memory serves, we rely on xmin of the pg_index
> >>>> row for purposes such as detecting whether a concurrently-created
> >>>> index is safe to use yet.
> >
> > I took a deeper look regarding this use case but was unable to find more evidence. As part of this patch, we
essentially
> make concurrently-created index safe to use only if transaction started after the xmin of Phase 3. Even today
concurrent
> indexes can not be used for transactions before this xmin because of the wait (which I am trying to get rid of in
this
> patch), is there any other denial of service you are talking about? Both the other states indislive, indisready can
> be transactional updates as far as I understand. Is there anything more I am missing here?
>
>
> Hi,
>
> I did some more concurrency testing here through some python scripts which compare the end state of the concurrently
> created indexes. I also back-ported this patch to PG 9.6 and ran some custom concurrency tests (Inserts, Deletes, and
> Create Index Concurrently) which seem to succeed. The intermediate states unfortunately are not easy to test in an
> automated manner, but to be fair concurrent indexes could never be used for older transactions. Do you have more
> inputs/ideas on this patch?

According to the commit 3c8404649 [1], transactional update in pg_index is not safe in non-MVCC catalog scans before
PG9.4.
But it seems to me that we can use transactional update in pg_index after the commit 813fb03155 [2] which got rid of
SnapshotNow. 

If we apply this patch back to 9.3 or earlier, we might need to consider another way or take the Andres suggestion
(whichI don't understand the way fully though), but which version do you want/do we need to apply this patch? 

Also, if we apply this patch in this way, there are several comments to be fixed which state the method of CREATE INDEX
CONCURRENTLY.

ex.
[index.c]
/*
* validate_index - support code for concurrent index builds
...
* After completing validate_index(), we wait until all transactions that
* were alive at the time of the reference snapshot are gone; this is
* necessary to be sure there are none left with a transaction snapshot
* older than the reference (and hence possibly able to see tuples we did
* not index).  Then we mark the index "indisvalid" and commit.  Subsequent
* transactions will be able to use it for queries.
...
valiate_index()
{
}


[1]
https://github.com/postgres/postgres/commit/3c84046490bed3c22e0873dc6ba492e02b8b9051#diff-b279fc6d56760ed80ce4178de1401d2c
[2]
https://github.com/postgres/postgres/commit/813fb0315587d32e3b77af1051a0ef517d187763#diff-b279fc6d56760ed80ce4178de1401d2c

--
Yoshikazu Imai