Re: Cleaning up historical portability baggage - Mailing list pgsql-hackers
From | Thomas Munro |
---|---|
Subject | Re: Cleaning up historical portability baggage |
Date | |
Msg-id | CA+hUKG+tqFVY7Fi=WBvZ6-UsATjcPNBDtphDm7YLjevm2kxSvw@mail.gmail.com Whole thread Raw |
In response to | Re: Cleaning up historical portability baggage (Andres Freund <andres@anarazel.de>) |
List | pgsql-hackers |
On Tue, Jun 10, 2025 at 2:25 AM Andres Freund <andres@anarazel.de> wrote: > On 2025-06-09 15:25:22 +0200, Michael Banck wrote: > > On Thu, Aug 11, 2022 at 10:02:29PM +1200, Thomas Munro wrote: > > > Remove configure probe for sys/uio.h. > > > > Removing the configure probe is fine, but the patch also changes > > behavior in the sense that IOV_MAX is now considered defined everywhere > > but on Windows. However, in the good-old GNU "we have no arbitrary > > limits" fashion, this breaks on GNU Hurd: > > > > |gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute-Wimplicit-fallthrough=3 -Wcast-function-type -Wshado > > |w=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation-Wno-stringop-truncation -g -O2 -DFRONTEND -I. -I../../src/common - > > |I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE -I/usr/include/libxml2\""-DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpoin > > |ter-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3-Wcast-function-type -Wshadow=compatible-local -Wformat-security > > |-fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2\"" -DVAL_CFLAGS_SL="\"-fPIC\""-DVAL_LDFLAGS="\"-Wl,--as-needed -Wl,-rpa > > |th,'/home/demo/build-farm-19.1/buildroot/REL_16_STABLE/inst/lib',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" -DVAL_LDFLAGS_SL="\"\""-DVAL_LIBS="\"-lpgcommon -lpgport -lxslt -lxml2 - > > |lssl -lcrypto -lgssapi_krb5 -lz -lreadline -lpthread -lm \"" -c -o file_utils.o file_utils.c > > |In file included from ../../src/include/postgres_fe.h:25, > > | from file_utils.c:19: > > |file_utils.c: In function 'pg_pwritev_with_retry': > > |../../src/include/port/pg_iovec.h:36:24: error: 'IOV_MAX' undeclared (first use in this function); did you mean 'INT_MAX'? > > | 36 | #define PG_IOV_MAX Min(IOV_MAX, 32) > > | | ^~~~~~~ > > |../../src/include/c.h:988:35: note: in definition of macro 'Min' > > | 988 | #define Min(x, y) ((x) < (y) ? (x) : (y)) > > | | ^ > > |file_utils.c:474:31: note: in expansion of macro 'PG_IOV_MAX' > > | 474 | struct iovec iov_copy[PG_IOV_MAX]; > > | | ^~~~~~~~~~ > > |../../src/include/port/pg_iovec.h:36:24: note: each undeclared identifier is reported only once for each function itappears in > > > > |$ grep IOV_MAX /usr/include/i386-gnu/bits/uio_lim.h > > |#undef __IOV_MAX > > > > Postgres built fine up and until v15 on the Hurd, so this is a build > > regression, and the fact that we define #PG_IOV_MAX to at most 32 > > anyway suggest we could just #definde IOV_MAX to 16 if undefined as on > > Windows. > > I think our policy basically is that if it doesn't exist on the BF, it's > unsupported. Also note that Hurd is not listed as a supported OS: > https://www.postgresql.org/docs/devel/supported-platforms.html > > We can't design for OS that we don't know it's used with postgres and/or how > that OS works / what it supports. > > So I reject the premise that this is a regression. > > If you want to argue that we should add support for Hurd, you can do that, but > that's obviously a different discussion. Since time immemorial, PostgreSQL could compile successfully on GNU/Hurd systems (perhaps with a few superficial tweak patches of this ilk), but it couldn't actually run because its semaphores didn't work cross-process. I knew that because Christoph told us somewhere on this list not so long ago, but it seems there has been some movement since then and I am guilty of being wrong on the internet, as mentioned here: https://savannah.gnu.org/task/?7050 So it looks like they added working semaphores in 2022, but in 2023 I removed some unrelated stuff during and happened to mention that Hurd can't run PostgreSQL anyway, and they didn't like that (I mean it wasn't an unreasonable chain of thought, it was just unfortunate timing given they'd worked on addressing that). The log linked there shows the regression tests mostly passing, so that seems promising: at least a cluster can finally actually start up and run! There may be a few more little things to chase down, who knows. I wonder if the pthreads implementation is still unfinished as hinted at there too. We're gonna need that as a hard dependency quite soon, or at least several of us are working hard to make that true... that said, we'd be far from the only thing needing pthreads so that must be a high priority for them if they haven't already addressed it. Anyway, that's a long way of saying +1, GNU/Hurd would be a new platform for us, and obviously it hasn't even been plausible until quite recently. There are also circularities when you're bringing up an OS and application support for it in parallel, so there will no doubt be a few little threads like this along the way, and I'm not against helping on our side. More than that, for the specific point, yeah the GNU project's unlimited limits thing is kinda admirable, and they are quite right to complain about this one: {IOV_MAX} appears with curly braces in POSIX and <limits.h> says: "A definition of one of the symbolic constants in the following list shall be omitted from <limits.h> on specific implementations where the corresponding value is equal to or greater than the stated minimum, but is unspecified." So irrespective of Hurd support, it was a mistake to require the symbol to be defined, based on POSIX language lawyering alone. It's annoying though, we've hit the only known system so far that doesn't actually define it, and POSIX then says we can only rely on 16 if we want a compile-time limit. Yet Hurd presumably really wants to make it unlimited (well limited only by things like data types), while POSIX says that if we want to use anything but the required minimum when the macro isn't defined, we should call sysconf(_SC_IOV_MAX) at runtime. So... I guess the correct thing to do may be to use 16 for the benefit of our (few) statically sized arrays that Richard Stallman would hate, but also consult sysconf() when limiting io_max_combine_limit, our own runtime bound on I/O combining for the main serious user of vectored I/O. A little complicated, but... they're quite right about all that. Hnghh.
pgsql-hackers by date: