Thread: pgsql: Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURR
Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY. CIC and REINDEX CONCURRENTLY assume backends see their catalog changes no later than each backend's next transaction start. That failed to hold when a backend absorbed a relevant invalidation in the middle of running RelationBuildDesc() on the CIC index. Queries that use the resulting index can silently fail to find rows. Fix this for future index builds by making RelationBuildDesc() loop until it finishes without accepting a relevant invalidation. It may be necessary to reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices. Back-patch to 9.6 (all supported versions). Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres Freund. Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/fdd965d074d46765c295223b119ca437dbcac973 Modified Files -------------- contrib/amcheck/t/002_cic.pl | 78 ++++++++++++++++ src/backend/utils/cache/inval.c | 12 ++- src/backend/utils/cache/relcache.c | 115 +++++++++++++++++++++-- src/bin/pgbench/t/001_pgbench_with_server.pl | 118 +++++++---------------- src/include/utils/inval.h | 1 + src/include/utils/relcache.h | 2 +- src/test/perl/PostgresNode.pm | 134 +++++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 8 files changed, 368 insertions(+), 93 deletions(-)
On 10/24/21 03:40, Noah Misch wrote: > Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY. > > CIC and REINDEX CONCURRENTLY assume backends see their catalog changes > no later than each backend's next transaction start. That failed to > hold when a backend absorbed a relevant invalidation in the middle of > running RelationBuildDesc() on the CIC index. Queries that use the > resulting index can silently fail to find rows. Fix this for future > index builds by making RelationBuildDesc() loop until it finishes > without accepting a relevant invalidation. It may be necessary to > reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices. > Back-patch to 9.6 (all supported versions). > > Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres > Freund. > > Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com > Unfortunately, this seems to have broken CLOBBER_CACHE_ALWAYS builds. Since this commit, initdb never completes due to infinite retrying over and over (on the first RelationBuildDesc call). We have a CLOBBER_CACHE_ALWAYS buildfarm machine "avocet", and that currently looks like this (top): PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 2626 buildfa+ 20 0 202888 21416 20084 R 98.34 0.531 151507:16 /home/buildfarm/avocet/buildroot/REL9_6_STABLE/pgsql.build/tmp_install/home/buildfarm/avocet/buildroot/REL9_6_STABLE/inst/bin/postgres --boot -x1 -F Yep, that's 151507 minutes, i.e. 104 days in initdb :-/ I haven't looked at this very closely yet, but it seems the whole problem is we do this at the very beginning: in_progress_list[in_progress_offset].invalidated = false; /* * find the tuple in pg_class corresponding to the given relation id */ pg_class_tuple = ScanPgRelation(targetRelId, true, false); which seems entirely self-defeating, because ScanPgRelation acquires a lock (on pg_class), which accepts invalidations, which invalidates system caches (in clobber_cache_always), which sets promptly sets in_progress_list[in_progress_offset].invalidated = false; guaranteeing an infinite loop. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: pgsql: Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURR
From
Andres Freund
Date:
Hi, On 2022-02-08 22:13:01 +0100, Tomas Vondra wrote: > On 10/24/21 03:40, Noah Misch wrote: > > Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY. > > > > CIC and REINDEX CONCURRENTLY assume backends see their catalog changes > > no later than each backend's next transaction start. That failed to > > hold when a backend absorbed a relevant invalidation in the middle of > > running RelationBuildDesc() on the CIC index. Queries that use the > > resulting index can silently fail to find rows. Fix this for future > > index builds by making RelationBuildDesc() loop until it finishes > > without accepting a relevant invalidation. It may be necessary to > > reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices. > > Back-patch to 9.6 (all supported versions). > > > > Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres > > Freund. > > > > Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com > > > > Unfortunately, this seems to have broken CLOBBER_CACHE_ALWAYS builds. Since > this commit, initdb never completes due to infinite retrying over and over > (on the first RelationBuildDesc call). Ugh. Do we need to do something about WRT the next set of minor releases? Is there a a chance of this occuring in "real" workloads? Greetings, Andres Freund