Date: Sat, 16 Dec 2017 20:06:05 -0800
From: Noah Misch <noah@leadboat.com>
To: pgsql-hackers@postgresql.org
Subject: Re: Race to build pg_isolation_regress in "make -j check-world"
Message-ID: <20171217040605.GA2421254@rfd.leadboat.com>
References: <20171106080752.GA1298146@rfd.leadboat.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="82I3+IH0IqGh5yIs"
Content-Disposition: inline
In-Reply-To: <20171106080752.GA1298146@rfd.leadboat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--82I3+IH0IqGh5yIs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Nov 06, 2017 at 12:07:52AM -0800, Noah Misch wrote:
> I've been enjoying the speed of parallel check-world, but I get spurious
> failures from makefile race conditions.  Commit c66b438 fixed the simple ones.
> More tricky is this problem of multiple "make" processes entering
> src/test/regress concurrently, which causes failures like these:
> 
>   gcc: error: pg_regress.o: No such file or directory
>   make[4]: *** [pg_isolation_regress] Error 1
> 
>   /bin/sh: ../../../src/test/isolation/pg_isolation_regress: Permission denied
>   make -C test_extensions check
>   make[2]: *** [check] Error 126
>   make[2]: Leaving directory `/home/nm/src/pg/backbranch/10/src/test/isolation'
> 
>   /bin/sh: ../../../../src/test/isolation/pg_isolation_regress: Text file busy
>   make[3]: *** [isolationcheck] Error 126
>   make[3]: Leaving directory `/home/nm/src/pg/backbranch/10/src/test/modules/snapshot_too_old'
> 
> This is reproducible since commit 2038bf4 or earlier; "make -j check-world"
> had worse problems before that era.  A workaround is to issue "make -j; make
> -j -C src/test/isolation" before the check-world.

Commit de0aca6 fixed that problem, but I now see similar trouble from multiple
"make" processes running "make -C contrib/test_decoding install" concurrently.
This is a risk for any directory named in an EXTRA_INSTALL variable of more
than one makefile.  Under the right circumstances, this would affect
contrib/hstore and others in addition to contrib/test_decoding.  That brings
me back to the locking idea:

> The problem of multiple "make" processes in a directory (especially src/port)
> shows up elsewhere.  In a cleaned tree, "make -j -C src/bin" or "make -j
> installcheck-world" will do it.  For more-prominent use cases, src/Makefile
> prevents this with ".NOTPARALLEL:" and building first the directories that are
> frequent submake targets.  Perhaps we could fix the general problem with
> directory locking; targets that call "$(MAKE) -C FOO" would first sleep until
> FOO's lock is available.  That could be tricky to make robust.

If one is willing to assume that a lock-holding process never crashes, locking
in a shell script is simple: mkdir to lock, rmdir to unlock.  I don't want to
assume that.  The bakery algorithm provides convenient opportunities for
checking whether the last locker crashed; I have attached a shell script
demonstrating this approach.  Better ideas?  Otherwise, I'll look into
integrating this design into the makefiles.

Thanks,
nm

--82I3+IH0IqGh5yIs
Content-Type: application/x-sh
Content-Disposition: attachment; filename="bakery.sh"
Content-Transfer-Encoding: quoted-printable

#! /bin/sh=0A=0A# Demonstrate use of the bakery algorithm for crash-safe mu=
tual exclusion in a=0A# shell script.  Sample invocation:=0A#   (rm -r buil=
dlock; for child in `seq 1 10`; do sh bakery.sh & done; wait)=0A=0Aset -u=
=0APS4=3D'+$$+ '  # include PID in "set -x" messages=0Asleep=3D':'  # busy =
wait, because "sleep .01" is a GNU extension=0Amkdir -p buildlock/choosing =
buildlock/number=0A=0A# print the contents of a file, or "0" if file is mis=
sing or empty=0Alockread() {=0A  result=3D`2>/dev/null cat "$1"`=0A  test -=
z "$result" && result=3D0=0A  echo "$result"=0A}=0A=0Afor iter in `seq 1 10=
0`; do=0A=0A# Algorithm and terminology from:=0A#   A New Solution of Dijks=
tra's Concurrent Programming Problem Communications=0A#   of the ACM 17, 8 =
(August 1974), 453-455.=0Acd buildlock=0Aecho 1 >choosing/$$=0Aecho "$$ ent=
ered the doorway"=0Amax=3D0=0Acd number=0Aecho 0 >$$=0Afor pid in *; do=0A =
 number=3D`lockread $pid`=0A  [ $number -gt $max ] && max=3D$number=0Adone=
=0Amy_number=3D$((1 + $max))=0Aecho $my_number >$$=0Acd ../choosing=0Aecho =
0 >$$=0Aecho "$$ in the bakery, number =3D $my_number"=0Afor pid in $(print=
f '%s\n' * | sort -n); do=0A  while [ `lockread $pid` =3D 1 ]; do=0A	2>/dev=
/null kill -0 "$pid" || { rm -f ../number/$pid; rm -f $pid; }; $sleep=0A  d=
one=0A  while :; do=0A	number=3D`lockread ../number/$pid`=0A	[ $number -ne =
0 -a \=0A	  \( $number -lt $my_number -o \=0A	  $number -eq $my_number -a $=
pid -lt $$ \) ] || break=0A	2>/dev/null kill -0 "$pid" || { rm -f ../number=
/$pid; rm -f $pid; }; $sleep=0A  done=0Adone=0Acd ../..=0Aecho "$$ in criti=
cal section"=0A# exit  # simulate crash=0Aecho "$$ leaving critical section=
"=0Arm -f buildlock/number/$$=0Arm -f buildlock/choosing/$$=0A=0Adone=0A
--82I3+IH0IqGh5yIs--