Fixing parallel make of libpq

Started by Dagfinn Ilmari Mannsåkerover 6 years ago8 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t41774
psql -h localhost -U postgres

Built from patchset v8 (message #8), July 28, 2026 at 03:03 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t41774_8 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t41774_8 && git checkout t41774_8

Patchset v8 (message #8) is on t41774_8

Jump to latest

Hi hackers,

I noticed to my annoyance that 'make -j4 -C src/interfaces/libpq'
doesn't work in a clean checkout, because it can't find
libpgcommon_shlib and libpgport_shlib. It looks like that's because the
submake-libpgport dependency is declared on the all-lib target, not on
the shlib itself. Moving it to SHLIB_PREREQS instead fixes it, patch
for which is attached.

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

Attachments:

0001-Make-libpgport-a-dependency-of-the-libpq-shlib-itsel.patchtext/x-diffDownload+2-3
#2Michael Paquier
michael@paquier.xyz
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: Fixing parallel make of libpq

On Wed, Jan 08, 2020 at 01:33:13PM +0000, Dagfinn Ilmari Mannsåker wrote:

I noticed to my annoyance that 'make -j4 -C src/interfaces/libpq'
doesn't work in a clean checkout, because it can't find
libpgcommon_shlib and libpgport_shlib. It looks like that's because the
submake-libpgport dependency is declared on the all-lib target, not on
the shlib itself. Moving it to SHLIB_PREREQS instead fixes it, patch
for which is attached.

Hmm. That logically makes sense. Isn't that a side effect of 7143b3e
then? Now, FWIW, I am not able to reproduce it here, after trying on
two different machines, various parallel job numbers (up to 32), and a
couple of dozen attempts. Perhaps somebody else can see the failures?
--
Michael

In reply to: Michael Paquier (#2)
Re: Fixing parallel make of libpq

Michael Paquier <michael@paquier.xyz> writes:

On Wed, Jan 08, 2020 at 01:33:13PM +0000, Dagfinn Ilmari Mannsåker wrote:

I noticed to my annoyance that 'make -j4 -C src/interfaces/libpq'
doesn't work in a clean checkout, because it can't find
libpgcommon_shlib and libpgport_shlib. It looks like that's because the
submake-libpgport dependency is declared on the all-lib target, not on
the shlib itself. Moving it to SHLIB_PREREQS instead fixes it, patch
for which is attached.

Hmm. That logically makes sense. Isn't that a side effect of 7143b3e
then? Now, FWIW, I am not able to reproduce it here, after trying on
two different machines, various parallel job numbers (up to 32), and a
couple of dozen attempts. Perhaps somebody else can see the failures?

It fails reliably for me on Debian Buster, with make 4.2.1-1.2and -j4.
The command to reproduce it is:

git clean -xfd && ./configure && make -Otarget -j4 -C src/interfaces/libpq/

Attached is the output of the `make` step with and without the patch.

- ilmari
--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

Attachments:

make-libpq-parallel.logtext/plainDownload
make-libpq-parallel-fixed.logtext/plainDownload
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dagfinn Ilmari Mannsåker (#3)
Re: Fixing parallel make of libpq

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

Michael Paquier <michael@paquier.xyz> writes:

Hmm. That logically makes sense. Isn't that a side effect of 7143b3e
then? Now, FWIW, I am not able to reproduce it here, after trying on
two different machines, various parallel job numbers (up to 32), and a
couple of dozen attempts. Perhaps somebody else can see the failures?

It fails reliably for me on Debian Buster, with make 4.2.1-1.2and -j4.

Yeah, it's also reliable for me on Fedora 30:

$ make -s clean
$ make -s -j4 -C src/interfaces/libpq
/usr/bin/ld: cannot find -lpgcommon_shlib
/usr/bin/ld: cannot find -lpgport_shlib
collect2: error: ld returned 1 exit status
make: *** [../../../src/Makefile.shlib:293: libpq.so.5.13] Error 1
make: *** Waiting for unfinished jobs....

On a RHEL6 box, the same test only draws a complaint about
-lpgcommon_shlib, so it does seem like there's some make version
dependency in here. And of course the whole thing is a race condition
anyway, so naturally it's going to be pretty context-sensitive.

My thoughts about the patch:

1) Changing from an "|"-style dependency to a plain dependency seems
like a semantics change. I've never been totally clear on the
difference though. I think Peter introduced our use of the "|" style,
so maybe he can comment.

2) The same coding pattern is used in a bunch of other places, so if
this spot is broken, there probably are a lot of others that need a
similar change. On the other hand, there may not be that many
directories that are likely places to start a parallel build from,
so maybe we don't care elsewhere.

regards, tom lane

In reply to: Tom Lane (#4)
Re: Fixing parallel make of libpq

Tom Lane <tgl@sss.pgh.pa.us> writes:

My thoughts about the patch:

1) Changing from an "|"-style dependency to a plain dependency seems
like a semantics change. I've never been totally clear on the
difference though. I think Peter introduced our use of the "|" style,
so maybe he can comment.

Makefile.shlib puts $(SHLIB_PREREQS) after the "|":

$ grep SHLIB_PREREQS src/Makefile.shlib
# SHLIB_PREREQS Order-only prerequisites for library build target
$(stlib): $(OBJS) | $(SHLIB_PREREQS)
$(shlib): $(OBJS) | $(SHLIB_PREREQS)
$(shlib): $(OBJS) | $(SHLIB_PREREQS)
$(shlib): $(OBJS) | $(SHLIB_PREREQS)
$(shlib): $(OBJS) | $(SHLIB_PREREQS)
$(shlib): $(OBJS) $(DLL_DEFFILE) | $(SHLIB_PREREQS)

2) The same coding pattern is used in a bunch of other places, so if
this spot is broken, there probably are a lot of others that need a
similar change. On the other hand, there may not be that many
directories that are likely places to start a parallel build from,
so maybe we don't care elsewhere.

Grepping the Makefiles for ':.*submake-' shows that they are on the
actual build artifact target, libpq was just the outlier having it on
the phony "all" target. For example pg_basebackup:

pg_basebackup: pg_basebackup.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
$(CC) $(CFLAGS) pg_basebackup.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)

pg_receivewal: pg_receivewal.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
$(CC) $(CFLAGS) pg_receivewal.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)

pg_recvlogical: pg_recvlogical.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
$(CC) $(CFLAGS) pg_recvlogical.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)

- ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: Fixing parallel make of libpq

On 2020-01-09 15:17, Tom Lane wrote:

1) Changing from an "|"-style dependency to a plain dependency seems
like a semantics change. I've never been totally clear on the
difference though. I think Peter introduced our use of the "|" style,
so maybe he can comment.

If you have a phony target as a prerequisite of a real-file target, you
should make that an order-only ("|") prerequisite. Otherwise the
real-file target rules will *always* be run, on account of the phony
target prerequisite.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#6)
Re: Fixing parallel make of libpq

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-01-09 15:17, Tom Lane wrote:

1) Changing from an "|"-style dependency to a plain dependency seems
like a semantics change. I've never been totally clear on the
difference though. I think Peter introduced our use of the "|" style,
so maybe he can comment.

If you have a phony target as a prerequisite of a real-file target, you
should make that an order-only ("|") prerequisite. Otherwise the
real-file target rules will *always* be run, on account of the phony
target prerequisite.

OK, got that. But that doesn't directly answer the question of whether
it's wrong to use a phony target as an order-only prerequisite of
another phony target. Grepping around for other possible issues,
I see that you recently added

update-unicode: | submake-generated-headers submake-libpgport
$(MAKE) -C src/common/unicode $@
$(MAKE) -C contrib/unaccent $@

Doesn't that also have parallel-make hazards, if libpq does?

regards, tom lane

In reply to: Tom Lane (#7)
Re: Fixing parallel make of libpq

Tom Lane <tgl@sss.pgh.pa.us> writes:

OK, got that. But that doesn't directly answer the question of whether
it's wrong to use a phony target as an order-only prerequisite of
another phony target. Grepping around for other possible issues,
I see that you recently added

update-unicode: | submake-generated-headers submake-libpgport
$(MAKE) -C src/common/unicode $@
$(MAKE) -C contrib/unaccent $@

Doesn't that also have parallel-make hazards, if libpq does?

The part of 'update-unicode' that needs the generated headers and
libpgport is src/common/unicode/norm_test, which is depended by on by
the normalization-check target in the same directory. Running 'make -C
src/common/unicode normalization-check' in a freshly-configured tree
does indeed fail, independent of parallelism or the update-unicode
target.

Adding the deps to the norm_test target fixes 'make -C
src/common/unicode normalization-check', but 'make -C src/common/unicode
update-unicode' still fails, because submake-generated-headers only does
its thing in the top-level make invocation, so that needs an explicit
dep as well.

Please find a patch attached. However, I don't think it's fair to block
fixing the actual libpq parallel-make bug that has bitten me several
times on auditing the entire source tree for vaguely related issues that
nobody has complained about yet.

- ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

Attachments:

t41774_8
0001-Add-missing-dependencies-in-src-common-unicode-Makef.patchtext/x-diffDownload+2-3