Hi Dhruv,
On Sun, June 30, 2019 at 7:30 AM, Goel, Dhruv wrote:
> > On Jun 10, 2019, at 1:20 PM, Goel, Dhruv <[email protected]> wrote:
> >> On Jun 9, 2019, at 5:33 PM, Tom Lane <[email protected]> wrote:
> >> Andres Freund <[email protected]> writes:
> >>> On June 9, 2019 8:36:37 AM PDT, Tom Lane <[email protected]> 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 (which I 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
