Re: pgsql: Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURR

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