Re: [HACKERS] orangutan seizes up during isolation-check
On 16 January 2015 at 01:33, Noah Misch wrote: > On Thu, Jan 15, 2015 at 09:24:01AM -0500, Robert Haas wrote: > > On Thu, Jan 15, 2015 at 1:04 AM, Noah Misch wrote: > > > On Wed, Jan 14, 2015 at 04:48:53PM -0500, Peter Eisentraut wrote: > > >> What I'm seeing now is that the unaccent regression tests when run > under > > >> make check-world abort with > > >> > > >> FATAL: postmaster became multithreaded during startup > > >> HINT: Set the LC_ALL environment variable to a valid locale. > > > > > > contrib/unaccent/Makefile sets NO_LOCALE=1, so that makes sense. I > expect the > > > patch over here will fix it: > > > > http://www.postgresql.org/message-id/20150109063015.ga2491...@tornado.leadboat.com > > > > I just hit this same problem; are you going to commit that patch soon? > > It's rather annoying to have make check-world fail. > > Sure, done. Dave, orangutan should now be able to pass with --enable-nls. > Would you restore that option? > I can, but is this for HEAD or all versions ?
Re: [HACKERS] orangutan seizes up during isolation-check
On Fri, Jan 16, 2015 at 05:43:44AM -0500, Dave Cramer wrote: > On 16 January 2015 at 01:33, Noah Misch wrote: > > Sure, done. Dave, orangutan should now be able to pass with --enable-nls. > > Would you restore that option? > > I can, but is this for HEAD or all versions ? All versions. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Thu, Jan 15, 2015 at 09:24:01AM -0500, Robert Haas wrote: > On Thu, Jan 15, 2015 at 1:04 AM, Noah Misch wrote: > > On Wed, Jan 14, 2015 at 04:48:53PM -0500, Peter Eisentraut wrote: > >> What I'm seeing now is that the unaccent regression tests when run under > >> make check-world abort with > >> > >> FATAL: postmaster became multithreaded during startup > >> HINT: Set the LC_ALL environment variable to a valid locale. > > > > contrib/unaccent/Makefile sets NO_LOCALE=1, so that makes sense. I expect > > the > > patch over here will fix it: > > http://www.postgresql.org/message-id/20150109063015.ga2491...@tornado.leadboat.com > > I just hit this same problem; are you going to commit that patch soon? > It's rather annoying to have make check-world fail. Sure, done. Dave, orangutan should now be able to pass with --enable-nls. Would you restore that option? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Thu, Jan 15, 2015 at 1:04 AM, Noah Misch wrote: > On Wed, Jan 14, 2015 at 04:48:53PM -0500, Peter Eisentraut wrote: >> What I'm seeing now is that the unaccent regression tests when run under >> make check-world abort with >> >> FATAL: postmaster became multithreaded during startup >> HINT: Set the LC_ALL environment variable to a valid locale. > > contrib/unaccent/Makefile sets NO_LOCALE=1, so that makes sense. I expect the > patch over here will fix it: > http://www.postgresql.org/message-id/20150109063015.ga2491...@tornado.leadboat.com I just hit this same problem; are you going to commit that patch soon? It's rather annoying to have make check-world fail. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Wed, Jan 14, 2015 at 04:48:53PM -0500, Peter Eisentraut wrote: > What I'm seeing now is that the unaccent regression tests when run under > make check-world abort with > > FATAL: postmaster became multithreaded during startup > HINT: Set the LC_ALL environment variable to a valid locale. contrib/unaccent/Makefile sets NO_LOCALE=1, so that makes sense. I expect the patch over here will fix it: http://www.postgresql.org/message-id/20150109063015.ga2491...@tornado.leadboat.com > The behavior is also a bit strange. The step > > == starting postmaster== > > hangs for one minute before failing. This probably has nothing to do > with your change, but maybe pg_regress could detect postmaster startup > failures better. Yeah, I think any postmaster startup failure has that effect. Not ideal. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On 1/1/15 11:04 PM, Noah Misch wrote: >> Clusters hosted on OS X fall into these categories: >> >> 1) Unaffected configuration. This includes everyone setting a valid messages >>locale via LANG, LC_ALL or LC_MESSAGES. >> 2) Affected configuration. Through luck and light use, the cluster would not >>experience the crashes/hangs. >> 3) Cluster would experience the crashes/hangs. >> >> DBAs in (3) want the FATAL at startup, but those in (2) want a LOG message >> instead. DBAs in (1) don't care. Since intermittent postmaster hangs are >> far >> worse than startup failure, if (2) and (3) have similar population, FATAL is >> the better bet. If (2) is sufficiently more populous than (3), then the many >> small pricks from startup failure do add up to hurt more than the occasional >> postmaster hang. Who knows how that calculation plays out. > > The first attached patch, for all branches, adds LOG-level messages and an > assertion. So cassert builds will fail hard, while others won't. The second > patch, for master only, changes the startup-time message to FATAL. If we > decide to use FATAL in all branches, I would just squash them into one. What I'm seeing now is that the unaccent regression tests when run under make check-world abort with FATAL: postmaster became multithreaded during startup HINT: Set the LC_ALL environment variable to a valid locale. My locale settings for this purpose are LANG="en_US.UTF-8" LC_COLLATE="en_US.UTF-8" LC_CTYPE="en_US.UTF-8" LC_MESSAGES="en_US.UTF-8" LC_MONETARY="en_US.UTF-8" LC_NUMERIC="en_US.UTF-8" LC_TIME="en_US.UTF-8" LC_ALL= The environment variable LANG is set, all the other ones are not set. I could presumably set LC_ALL, but then I lose the ability to set different locales for different categories. Running LC_ALL=$LANG make -C contrib/unaccent check doesn't fix it; I get the same error. The behavior is also a bit strange. The step == starting postmaster== hangs for one minute before failing. This probably has nothing to do with your change, but maybe pg_regress could detect postmaster startup failures better. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Wed, Dec 31, 2014 at 01:52:49PM -0500, Noah Misch wrote:
> On Sun, Dec 28, 2014 at 07:20:04PM -0500, Andrew Dunstan wrote:
> > On 12/28/2014 04:58 PM, Noah Misch wrote:
> > >The gettext maintainer was open to implementing the
> > >setlocale_native_forked()
> > >technique in gettext, though the last visible progress was in October. In
> > >any
> > >event, PostgreSQL builds will see older gettext for several years. If
> > >setlocale-darwin-fork-v1.patch is not wanted, I suggest making the
> > >postmaster
> > >check during startup whether it has become multithreaded. If
> > >multithreaded:
> > >
> > > FATAL: postmaster became multithreaded during startup
> > > HINT: Set the LC_ALL environment variable to a valid locale.
>
> > >I would like to go ahead and commit setlocale-main-harden-v1.patch, which
> > >is a
> > >good thing to have regardless of what happens with gettext.
> > >
> >
> > I'm OK with this, but on its own it won't fix orangutan's problems, will it?
>
> Right; setlocale-main-harden-v1.patch fixes a bug not affecting orangutan at
> all. None of the above will make orangutan turn green. Checking
> multithreading during startup would merely let it fail cleanly.
OS X --enable-nls buildfarm members should run tests under LANG=C instead of
with locale environment variables unset (make check NO_LOCALE=1). I see two
ways to arrange that: (1) add a build-farm.conf option, or (2) have
pg_regress.c:initialize_environment() treat OS X like Windows. I mildly favor
(2); see attached, untested patch. Windows and OS X --enable-nls share the
characteristic that setlocale(LC_x, "") consults sources other than
environment variables. (I do wonder why commit 4a6fd46 used LANG=en instead
of LANG=C.) On the other hand, LANG=en has been inessential on Windows ever
since "pg_regress --no-locale" started to use "initdb --no-locale". While I
prefer to see the LANG= hack go away rather than proliferate, I can't cite a
practical reason to care.
Thanks,
nm
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index e8c644b..e55835e 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -790,9 +790,16 @@ initialize_environment(void)
unsetenv("LC_NUMERIC");
unsetenv("LC_TIME");
unsetenv("LANG");
- /* On Windows the default locale cannot be English, so force it
*/
-#if defined(WIN32) || defined(__CYGWIN__)
- putenv("LANG=en");
+ /*
+* Most platforms have adopted the POSIX locale as their
+* implementation-defined default locale. Exceptions include
native
+* Windows, Darwin with --enable-nls, and Cygwin with
--enable-nls.
+* (Use of --enable-nls matters because libintl replaces
setlocale().)
+* Also, PostgreSQL does not support Darwin with locale
environment
+* variables unset; see PostmasterMain().
+*/
+#if defined(WIN32) || defined(__CYGWIN__) || defined(__darwin__)
+ putenv("LANG=C");
#endif
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Mon, Jan 05, 2015 at 02:25:09PM +0900, Michael Paquier wrote:
> On Fri, Jan 2, 2015 at 1:04 PM, Noah Misch wrote:
> > The first attached patch, for all branches, adds LOG-level messages and an
> > assertion. So cassert builds will fail hard, while others won't. The
> > second
> > patch, for master only, changes the startup-time message to FATAL. If we
> > decide to use FATAL in all branches, I would just squash them into one.
>
> + errdetail("Please report this to .")));
> Er, is mentioning a mailing list in an error message really necessary?
Necessary? No.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Fri, Jan 2, 2015 at 1:04 PM, Noah Misch wrote:
> The first attached patch, for all branches, adds LOG-level messages and an
> assertion. So cassert builds will fail hard, while others won't. The second
> patch, for master only, changes the startup-time message to FATAL. If we
> decide to use FATAL in all branches, I would just squash them into one.
+ errdetail("Please report this to .")));
Er, is mentioning a mailing list in an error message really necessary?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Wed, Dec 31, 2014 at 01:56:08PM -0500, Noah Misch wrote:
> On Wed, Dec 31, 2014 at 12:32:37AM -0500, Robert Haas wrote:
> > On Sun, Dec 28, 2014 at 4:58 PM, Noah Misch wrote:
> > > I wondered whether to downgrade FATAL to LOG in back branches.
> > > Introducing a
> > > new reason to block startup is disruptive for a minor release, but having
> > > the
> > > postmaster deadlock at an unpredictable later time is even more
> > > disruptive. I
> > > am inclined to halt startup that way in all branches.
> >
> > Jeepers. I'd rather not do that. From your report, this problem has
> > been around for years. Yet, as far as I know, it's bothering very few
> > real users, some of whom might be far more bothered by the postmaster
> > suddenly failing to start. I'm fine with a FATAL in master, but I'd
> > vote against doing anything that might prevent startup in the
> > back-branches without more compelling justification.
>
> Clusters hosted on OS X fall into these categories:
>
> 1) Unaffected configuration. This includes everyone setting a valid messages
>locale via LANG, LC_ALL or LC_MESSAGES.
> 2) Affected configuration. Through luck and light use, the cluster would not
>experience the crashes/hangs.
> 3) Cluster would experience the crashes/hangs.
>
> DBAs in (3) want the FATAL at startup, but those in (2) want a LOG message
> instead. DBAs in (1) don't care. Since intermittent postmaster hangs are far
> worse than startup failure, if (2) and (3) have similar population, FATAL is
> the better bet. If (2) is sufficiently more populous than (3), then the many
> small pricks from startup failure do add up to hurt more than the occasional
> postmaster hang. Who knows how that calculation plays out.
The first attached patch, for all branches, adds LOG-level messages and an
assertion. So cassert builds will fail hard, while others won't. The second
patch, for master only, changes the startup-time message to FATAL. If we
decide to use FATAL in all branches, I would just squash them into one.
diff --git a/configure b/configure
index 7594401..f7dedc0 100755
--- a/configure
+++ b/configure
@@ -11366,7 +11366,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit
mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask
symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
+for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit
mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle
setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes
wcstombs wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index 0dc3f18..76c6405 100644
--- a/configure.in
+++ b/configure.in
@@ -1265,7 +1265,7 @@ PGAC_FUNC_GETTIMEOFDAY_1ARG
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit
mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask
symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
+AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit
mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle
setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes
wcstombs wcstombs_l])
AC_REPLACE_FUNCS(fseeko)
case $host_os in
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 5106f52..6b0190c 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -87,6 +87,10 @@
#include
#endif
+#ifdef HAVE_PTHREAD_IS_THREADED_NP
+#include
+#endif
+
#include "access/transam.h"
#include "access/xlog.h"
#include "bootstrap/bootstrap.h"
@@ -1200,6 +1204,24 @@ PostmasterMain(int argc, char *argv[])
*/
RemovePgTempFiles();
+#ifdef HAVE_PTHREAD_IS_THREADED_NP
+
+ /*
+* On Darwin, libintl replaces setlocale() with a version that calls
+* CFLocaleCopyCurrent() when its second argument is "" and every
relevant
+* environment variable is unset or empty. CFLocaleCopyCurrent() makes
+* the process multithreaded. The postmaster calls sigprocmask() and
+* calls fork() without an immediate exec(), both of which have
undefined
+* behavior in a multithreaded program. A multithreaded postmaster is
the
+* normal case on Windows, which offers neither fork() nor
sigprocmask().
+*/
+ if (pthread_is_threaded_np() != 0)
+ ereport(LOG,
+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+errmsg("postmaster became multithreaded during
startup"),
Re: [HACKERS] orangutan seizes up during isolation-check
On Wed, Dec 31, 2014 at 01:55:23PM -0500, Dave Cramer wrote: > So at this point removing the --enable-nls from my config will solve the > build problem. > > Everyone knows there is an issue so there is no point in continuing to have > it fail. We hope all packagers will build with --enable-nls, so OS X buildfarm coverage thereof will be valuable. Let me see what I can pull together over the next several weeks toward getting it green. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
So at this point removing the --enable-nls from my config will solve the build problem. Everyone knows there is an issue so there is no point in continuing to have it fail. On 31 December 2014 at 13:52, Noah Misch wrote: > On Sun, Dec 28, 2014 at 07:20:04PM -0500, Andrew Dunstan wrote: > > On 12/28/2014 04:58 PM, Noah Misch wrote: > > >The gettext maintainer was open to implementing the > setlocale_native_forked() > > >technique in gettext, though the last visible progress was in October. > In any > > >event, PostgreSQL builds will see older gettext for several years. If > > >setlocale-darwin-fork-v1.patch is not wanted, I suggest making the > postmaster > > >check during startup whether it has become multithreaded. If > multithreaded: > > > > > > FATAL: postmaster became multithreaded during startup > > > HINT: Set the LC_ALL environment variable to a valid locale. > > > >I would like to go ahead and commit setlocale-main-harden-v1.patch, > which is a > > >good thing to have regardless of what happens with gettext. > > > > > > > I'm OK with this, but on its own it won't fix orangutan's problems, will > it? > > Right; setlocale-main-harden-v1.patch fixes a bug not affecting orangutan > at > all. None of the above will make orangutan turn green. Checking > multithreading during startup would merely let it fail cleanly. >
Re: [HACKERS] orangutan seizes up during isolation-check
On Wed, Dec 31, 2014 at 12:32:37AM -0500, Robert Haas wrote: > On Sun, Dec 28, 2014 at 4:58 PM, Noah Misch wrote: > > I wondered whether to downgrade FATAL to LOG in back branches. Introducing > > a > > new reason to block startup is disruptive for a minor release, but having > > the > > postmaster deadlock at an unpredictable later time is even more disruptive. > > I > > am inclined to halt startup that way in all branches. > > Jeepers. I'd rather not do that. From your report, this problem has > been around for years. Yet, as far as I know, it's bothering very few > real users, some of whom might be far more bothered by the postmaster > suddenly failing to start. I'm fine with a FATAL in master, but I'd > vote against doing anything that might prevent startup in the > back-branches without more compelling justification. Clusters hosted on OS X fall into these categories: 1) Unaffected configuration. This includes everyone setting a valid messages locale via LANG, LC_ALL or LC_MESSAGES. 2) Affected configuration. Through luck and light use, the cluster would not experience the crashes/hangs. 3) Cluster would experience the crashes/hangs. DBAs in (3) want the FATAL at startup, but those in (2) want a LOG message instead. DBAs in (1) don't care. Since intermittent postmaster hangs are far worse than startup failure, if (2) and (3) have similar population, FATAL is the better bet. If (2) is sufficiently more populous than (3), then the many small pricks from startup failure do add up to hurt more than the occasional postmaster hang. Who knows how that calculation plays out. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Sun, Dec 28, 2014 at 07:20:04PM -0500, Andrew Dunstan wrote: > On 12/28/2014 04:58 PM, Noah Misch wrote: > >The gettext maintainer was open to implementing the setlocale_native_forked() > >technique in gettext, though the last visible progress was in October. In > >any > >event, PostgreSQL builds will see older gettext for several years. If > >setlocale-darwin-fork-v1.patch is not wanted, I suggest making the postmaster > >check during startup whether it has become multithreaded. If multithreaded: > > > > FATAL: postmaster became multithreaded during startup > > HINT: Set the LC_ALL environment variable to a valid locale. > >I would like to go ahead and commit setlocale-main-harden-v1.patch, which is > >a > >good thing to have regardless of what happens with gettext. > > > > I'm OK with this, but on its own it won't fix orangutan's problems, will it? Right; setlocale-main-harden-v1.patch fixes a bug not affecting orangutan at all. None of the above will make orangutan turn green. Checking multithreading during startup would merely let it fail cleanly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Sun, Dec 28, 2014 at 4:58 PM, Noah Misch wrote: > I wondered whether to downgrade FATAL to LOG in back branches. Introducing a > new reason to block startup is disruptive for a minor release, but having the > postmaster deadlock at an unpredictable later time is even more disruptive. I > am inclined to halt startup that way in all branches. Jeepers. I'd rather not do that. From your report, this problem has been around for years. Yet, as far as I know, it's bothering very few real users, some of whom might be far more bothered by the postmaster suddenly failing to start. I'm fine with a FATAL in master, but I'd vote against doing anything that might prevent startup in the back-branches without more compelling justification. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On 12/28/2014 04:58 PM, Noah Misch wrote: On Sat, Oct 11, 2014 at 09:07:46AM -0400, Peter Eisentraut wrote: On 10/11/14 1:41 AM, Noah Misch wrote: Good question. It would be nice to make the change there, for the benefit of other consumers. The patch's setlocale_native_forked() assumes it never runs in a multithreaded process, but libintl_setlocale() must not assume that. I see a few ways libintl/gnulib might proceed: Yeah, it's difficult to see how they might proceed if they keep calling into Core Foundation, which might do anything, now or in the future. I went ahead and submitted a bug report to gettext: https://savannah.gnu.org/bugs/index.php?43404 (They way I understand it is that the files concerned originate in gettext and are copied to gnulib.) Let's see what they say. The gettext maintainer was open to implementing the setlocale_native_forked() technique in gettext, though the last visible progress was in October. In any event, PostgreSQL builds will see older gettext for several years. If setlocale-darwin-fork-v1.patch is not wanted, I suggest making the postmaster check during startup whether it has become multithreaded. If multithreaded: FATAL: postmaster became multithreaded during startup HINT: Set the LC_ALL environment variable to a valid locale. I wondered whether to downgrade FATAL to LOG in back branches. Introducing a new reason to block startup is disruptive for a minor release, but having the postmaster deadlock at an unpredictable later time is even more disruptive. I am inclined to halt startup that way in all branches. Yeah. It should be easily fixable, AIUI, and startup is surely a good and obvious time to to that. I like the idea of calling pthread_is_threaded_np() as a verification. This appears to be a OS X-specific function at the moment. If other platforms start adding it, then we'll run into the usual problems of how to link binaries that use pthread functions. Maybe that's not a realistic concern. True. As written, "configure" will report the function unavailable if it requires threading libraries. For a measure that's just a backstop against other bugs, that may be just right. I would like to go ahead and commit setlocale-main-harden-v1.patch, which is a good thing to have regardless of what happens with gettext. I'm OK with this, but on its own it won't fix orangutan's problems, will it? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Sat, Oct 11, 2014 at 09:07:46AM -0400, Peter Eisentraut wrote: > On 10/11/14 1:41 AM, Noah Misch wrote: > > Good question. It would be nice to make the change there, for the benefit > > of > > other consumers. The patch's setlocale_native_forked() assumes it never > > runs > > in a multithreaded process, but libintl_setlocale() must not assume that. I > > see a few ways libintl/gnulib might proceed: > > Yeah, it's difficult to see how they might proceed if they keep calling > into Core Foundation, which might do anything, now or in the future. > > I went ahead and submitted a bug report to gettext: > https://savannah.gnu.org/bugs/index.php?43404 > > (They way I understand it is that the files concerned originate in > gettext and are copied to gnulib.) > > Let's see what they say. The gettext maintainer was open to implementing the setlocale_native_forked() technique in gettext, though the last visible progress was in October. In any event, PostgreSQL builds will see older gettext for several years. If setlocale-darwin-fork-v1.patch is not wanted, I suggest making the postmaster check during startup whether it has become multithreaded. If multithreaded: FATAL: postmaster became multithreaded during startup HINT: Set the LC_ALL environment variable to a valid locale. I wondered whether to downgrade FATAL to LOG in back branches. Introducing a new reason to block startup is disruptive for a minor release, but having the postmaster deadlock at an unpredictable later time is even more disruptive. I am inclined to halt startup that way in all branches. > I like the idea of calling pthread_is_threaded_np() as a verification. > This appears to be a OS X-specific function at the moment. If other > platforms start adding it, then we'll run into the usual problems of how > to link binaries that use pthread functions. Maybe that's not a > realistic concern. True. As written, "configure" will report the function unavailable if it requires threading libraries. For a measure that's just a backstop against other bugs, that may be just right. I would like to go ahead and commit setlocale-main-harden-v1.patch, which is a good thing to have regardless of what happens with gettext. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On 10/11/14 1:41 AM, Noah Misch wrote: > Good question. It would be nice to make the change there, for the benefit of > other consumers. The patch's setlocale_native_forked() assumes it never runs > in a multithreaded process, but libintl_setlocale() must not assume that. I > see a few ways libintl/gnulib might proceed: Yeah, it's difficult to see how they might proceed if they keep calling into Core Foundation, which might do anything, now or in the future. I went ahead and submitted a bug report to gettext: https://savannah.gnu.org/bugs/index.php?43404 (They way I understand it is that the files concerned originate in gettext and are copied to gnulib.) Let's see what they say. I like the idea of calling pthread_is_threaded_np() as a verification. This appears to be a OS X-specific function at the moment. If other platforms start adding it, then we'll run into the usual problems of how to link binaries that use pthread functions. Maybe that's not a realistic concern. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Fri, Oct 10, 2014 at 09:14:38PM -0400, Peter Eisentraut wrote: > On 10/10/14 8:24 PM, Noah Misch wrote: > > Here's an implementation thereof covering both backend and frontend use of > > setlocale(). A setlocale() wrapper, pg_setlocale(), injects use of the > > child > > process where necessary. > > Would such a change not be better in gnulib itself? Good question. It would be nice to make the change there, for the benefit of other consumers. The patch's setlocale_native_forked() assumes it never runs in a multithreaded process, but libintl_setlocale() must not assume that. I see a few ways libintl/gnulib might proceed: 1) exec() a helper program to run CFLocaleCopyCurrent(). Distributing that executable alongside libintl and locating it at runtime is daunting. 2) Audit the CFLocaleCopyCurrent() code to see if it will, in practice, run reliably in a fork of a multithreaded process. (That conclusion could change without notice.) Use the setlocale_native_forked() technique. 3) Don't change libintl_setlocale(), but add a libintl_setlocale_nothread() for single-threaded consumers to adopt. Each consumer will need to modify code. libintl has a lean API; I expect adding this would be controversial. Among those, I would probably adopt (2). We know much about the process conditions in which pg_setlocale() will run, so placing the workaround in PostgreSQL erases the problem of (2). I'm inclined to stick with placing the workaround in PostgreSQL, but there are fair arguments on both sides. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On 10/10/14 8:24 PM, Noah Misch wrote: > Here's an implementation thereof covering both backend and frontend use of > setlocale(). A setlocale() wrapper, pg_setlocale(), injects use of the child > process where necessary. Would such a change not be better in gnulib itself? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Mon, Sep 15, 2014 at 12:51:14AM -0400, Noah Misch wrote: > 1. Fork, call setlocale(LC_x, "") in the child, pass back the effective locale >name through a pipe, and pass that name to setlocale() in the original >process. The short-lived child will get the extra threads, and the >postmaster will remain clean. Here's an implementation thereof covering both backend and frontend use of setlocale(). A setlocale() wrapper, pg_setlocale(), injects use of the child process where necessary. I placed the new function in src/common/exec.c, because every executable already needed that file's code and translatable strings for set_pglocale_pgservice() and callees. (Some executables do miss exec.c translations; I did not update them.) src/port/chklocale.c was closer in subject matter, but libpq imports it; this function does not belong in libpq. Also, this function would benefit from a frontend ereport() implementation, which is likely to land in libpgcommon if it lands at all. The runtime changes are conditional on __darwin__ but not on --enable-nls. NLS builds are the production norm; I'd like non-NLS builds to consistently exercise this code rather than shave its bytes and cycles. pg_setlocale() relies on main() setting all six LC_ environment variables. The second attached patch seals the cracks in main()'s longstanding attempt to do so. This improves preexisting user-visible behavior in weird cases: "LANG=pt_BR.utf8 LC_ALL=invalid postgres -D nosuch" will now print untranslated text, not pt_BR text. Documentation for each of lc_monetary, lc_numeric and lc_time says "If this variable is set to the empty string (which is the default) then the value is inherited from the execution environment of the server in a system-dependent way." Not so; by design, setlocale(LC_x, "") just re-selects the last value L passed to pg_perm_setlocale(LC_x, L). I have not touched this; I mention it for the sake of the archives. commit cd27ff7 (HEAD) Author: Noah Misch AuthorDate: Fri Oct 10 04:04:03 2014 -0400 Commit: Noah Misch CommitDate: Fri Oct 10 04:04:03 2014 -0400 When setlocale(LC_x, "") will start a thread, run it in a child process. Only Darwin, --enable-nls builds use a setlocale() that can start a thread. Buildfarm member orangutan experienced BackendList corruption on account of different postmaster threads executing signal handlers simultaneously. Furthermore, a multithreaded postmaster risks undefined behavior from sigprocmask() and fork(). Introduce pg_setlocale(), a wrapper around setlocale(). When the corresponding raw setlocale() call could start a thread, it forks, retrieves the setlocale() return value from the child, and uses that non-"" locale value to make an equivalent setlocale() call in the original process. The short-lived child does become multithreaded, but it uses no feature for which that poses a problem. Use of pg_setlocale() in the frontend protects forking executables, such as pg_dump, from undefined behavior. As the usage guideline comment implies, whether to call setlocale() or pg_setlocale() is a mere style question for most code sites. Opt for pg_setlocale() at indifferent code sites, leaving raw setlocale() calls in src/interfaces, in pg_setlocale() itself, and in callees of pg_setlocale(). Back-patch to 9.0 (all supported versions). diff --git a/configure b/configure index f0580ce..ee08963 100755 --- a/configure +++ b/configure @@ -11298,7 +11298,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l +for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.in b/configure.in index 527b076..2f8adfc 100644 --- a/configure.in +++ b/configure.in @@ -1257,7 +1257,7 @@ PGAC_FUNC_GETTIMEOFDAY_1ARG LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) +AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) AC_REPLACE_FUNCS(fseeko) c
Re: [HACKERS] orangutan seizes up during isolation-check
On Mon, Sep 15, 2014 at 10:11:57AM +0300, Heikki Linnakangas wrote: > On 09/15/2014 07:51 AM, Noah Misch wrote: > >libintl replaces setlocale(). Its setlocale(LC_x, "") uses OS-specific APIs > >to determine the default locale when $LANG and similar environment variables > >are empty, as they are during "make check NO_LOCALE=1". On OS X, it calls[1] > >CFLocaleCopyCurrent(), which in turn spins up a thread. See the end of this > >message for the postmaster thread stacks active upon hitting a breakpoint set > >at _dispatch_mgr_thread. > > Ugh. I'd call that a bug in libintl. setlocale() has no business to > make the process multi-threaded. Fair interpretation. libintl's options for mitigating the problem are even more constrained, unfortunately. > Do we have the same problem in backends? At a quick glance, aside > from postmaster we only use PG_SETMASK(&BlockSig) in signal > handlers, to prevent another signal handler from running > concurrently. In backends, we pass a specific value, not "", to pg_perm_setlocale(). (I expect the problem in EXEC_BACKEND builds, but I doubt anyone uses that as a production configuration on OS X.) Every frontend program does call setlocale(LC_ALL, "") via set_pglocale_pgservice(). None use sigprocmask(), but a few do use fork(). > >1. Fork, call setlocale(LC_x, "") in the child, pass back the effective > >locale > >name through a pipe, and pass that name to setlocale() in the original > >process. The short-lived child will get the extra threads, and the > >postmaster will remain clean. > >I'm skeptical of the value of looking up locale information using other OS X > >facilities when the usual environment variables are inconclusive, but I see > >no > >clear cause to reverse that decision now. I lean toward (1). > > Both of those are horrible hacks. And who's to say that calling > setlocale(LC_x, "foo") won't also call some function that makes the > process multi-threaded. If not in any current OS X release, it might > still happen in a future one. Right. Not just setlocale(); a large body of APIs could change that way. The CAVEATS section[1] of the OS X fork() manual page suggests Apple had that in mind at some point. To be 100% safe, we would write the postmaster as though it's always multithreaded, including making[2] EXEC_BACKEND the only supported configuration on OS X. Assuming we don't do that anytime soon, I did have in mind to make the postmaster raise an error if it becomes multithreaded. (Apple's pthread_is_threaded_np() is usable for this check.) Then, at least, we'll more-easily hear about new problems of this nature. > One idea would be to use an extra pthread mutex or similar, in > addition to PG_SETMASK(). Whenever you do PG_SETMASK(&BlockSig), > also grab the mutex, and release it when you do > PG_SETMASK(&UnBlockSig). I had considered and tested such a thing, and it sufficed to clear orangutan's present troubles. You still face undefined behavior after fork(). It's okay in practice if libdispatch threads are careful to register pthread_atfork() handlers for any relevant resources. It's a fair option, but running setlocale(LC_x, "") in a short-lived child assumes less about the system library implementation, seems no more of a hack to me, and isolates the overhead at postmaster start. [1] https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/fork.2.html [2] http://www.postgresql.org/message-id/1349280184-sup-...@alvh.no-ip.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On 09/15/2014 07:51 AM, Noah Misch wrote: libintl replaces setlocale(). Its setlocale(LC_x, "") uses OS-specific APIs to determine the default locale when $LANG and similar environment variables are empty, as they are during "make check NO_LOCALE=1". On OS X, it calls[1] CFLocaleCopyCurrent(), which in turn spins up a thread. See the end of this message for the postmaster thread stacks active upon hitting a breakpoint set at _dispatch_mgr_thread. Ugh. I'd call that a bug in libintl. setlocale() has no business to make the process multi-threaded. Do we have the same problem in backends? At a quick glance, aside from postmaster we only use PG_SETMASK(&BlockSig) in signal handlers, to prevent another signal handler from running concurrently. I see two options for fixing this in pg_perm_setlocale(LC_x, ""): 1. Fork, call setlocale(LC_x, "") in the child, pass back the effective locale name through a pipe, and pass that name to setlocale() in the original process. The short-lived child will get the extra threads, and the postmaster will remain clean. 2. On OS X, check for relevant environment variables. Finding none, set LC_x=C before calling setlocale(LC_x, ""). A variation is to raise ereport(FATAL) if sufficient environment variables aren't in place. Either way ensures the libintl setlocale() will never call CFLocaleCopyCurrent(). This is simpler than (1), but it entails a behavior change: "LANG= initdb" will use LANG=C or fail rather than use the OS X user account locale. I'm skeptical of the value of looking up locale information using other OS X facilities when the usual environment variables are inconclusive, but I see no clear cause to reverse that decision now. I lean toward (1). Both of those are horrible hacks. And who's to say that calling setlocale(LC_x, "foo") won't also call some function that makes the process multi-threaded. If not in any current OS X release, it might still happen in a future one. One idea would be to use an extra pthread mutex or similar, in addition to PG_SETMASK(). Whenever you do PG_SETMASK(&BlockSig), also grab the mutex, and release it when you do PG_SETMASK(&UnBlockSig). It would be nice to stop doing non-trivial things in the signal handler in the first place. It's pretty scary, even though it works when the process is single-threaded. I believe the reason it's currently implemented like that are the same problems that the latch code solves with the self-pipe trick: select() is not interrupted by a signal on all platforms, and even if it was, you would need pselect() with is not available (or does not work correctly even if it exists) on all platforms. I think we could use a latch in postmaster too. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] orangutan seizes up during isolation-check
On Tue, Sep 02, 2014 at 12:25:39AM -0400, Tom Lane wrote: > Noah Misch writes: > > Buildfarm member orangutan has failed chronically on both of the branches > > for > > which it still reports, HEAD and REL9_1_STABLE, for over two years. The > > postmaster appears to jam during isolation-check. Dave, orangutan currently > > has one such jammed postmaster for each branch. Could you gather some > > information about the running processes? > > What's particularly odd is that orangutan seems to be running an only > slightly out-of-date OS X release, which is hardly an unusual > configuration. My own laptop gets through isolation-check just fine. > Seems like there must be something nonstandard about orangutan's > software ... but what? Agreed. The difference is durable across OS X releases, because orangutan showed like symptoms under 10.7.3. Dave assisted me off-list with data collection and experimentation. Ultimately, --enable-nls was the key distinction, the absence of which spares the other OS X buildfarm animals. The explanation for ECONNREFUSED was more pedestrian than the reasons I had guessed. There were no jammed postmasters running as of the above writing. Rather, the postmasters were gone, but the socket directory entries remained. That happens when the postmaster suffers a "kill -9", a SIGSEGV, an assertion failure, or a similar abrupt exit. When I reproduced the problem, CountChildren() was attempting to walk a corrupt BackendList. Sometimes, the list had an entry such that e->next == e; these send CountChildren() into an infinite loop. Other times, testing "if (bp->dead_end)" prompted a segfault. That explains orangutan sometimes failing quickly and other times hanging for hours. Every crash showed at least two threads running in the postmaster. Multiple threads bring trouble in the form of undefined behavior for fork() w/o exec() and for sigprocmask(). The postmaster uses sigprocmask() to block most signals when doing something nontrivial; this allows it to do such nontrivial work in signal handlers. A sequence of 74 buildfarm runs caught 27 cases of a secondary thread running a signal handler, 14 cases of two signal handlers running at once, and one user-visible postmaster failure. libintl replaces setlocale(). Its setlocale(LC_x, "") uses OS-specific APIs to determine the default locale when $LANG and similar environment variables are empty, as they are during "make check NO_LOCALE=1". On OS X, it calls[1] CFLocaleCopyCurrent(), which in turn spins up a thread. See the end of this message for the postmaster thread stacks active upon hitting a breakpoint set at _dispatch_mgr_thread. I see two options for fixing this in pg_perm_setlocale(LC_x, ""): 1. Fork, call setlocale(LC_x, "") in the child, pass back the effective locale name through a pipe, and pass that name to setlocale() in the original process. The short-lived child will get the extra threads, and the postmaster will remain clean. 2. On OS X, check for relevant environment variables. Finding none, set LC_x=C before calling setlocale(LC_x, ""). A variation is to raise ereport(FATAL) if sufficient environment variables aren't in place. Either way ensures the libintl setlocale() will never call CFLocaleCopyCurrent(). This is simpler than (1), but it entails a behavior change: "LANG= initdb" will use LANG=C or fail rather than use the OS X user account locale. I'm skeptical of the value of looking up locale information using other OS X facilities when the usual environment variables are inconclusive, but I see no clear cause to reverse that decision now. I lean toward (1). Thanks, nm [1] http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/localename.c;h=78dc344bba191417855670fb751210d3608db6e6;hb=HEAD#l2883 thread #1: tid = 0xeccea9, 0x7fff9066b372 libsystem_notify.dylib`notify_register_check + 30, queue = 'com.apple.main-thread' frame #0: 0x7fff9066b372 libsystem_notify.dylib`notify_register_check + 30 frame #1: 0x7fff987cf261 libsystem_info.dylib`__si_module_static_ds_block_invoke + 109 frame #2: 0x7fff944d628d libdispatch.dylib`_dispatch_client_callout + 8 frame #3: 0x7fff944d61fc libdispatch.dylib`dispatch_once_f + 79 frame #4: 0x7fff987cf1f2 libsystem_info.dylib`si_module_static_ds + 42 frame #5: 0x7fff987cec65 libsystem_info.dylib`si_module_with_name + 60 frame #6: 0x7fff987cf0e7 libsystem_info.dylib`si_module_config_modules_for_category + 168 frame #7: 0x7fff987cedbd libsystem_info.dylib`__si_module_static_search_block_invoke + 87 frame #8: 0x7fff944d628d libdispatch.dylib`_dispatch_client_callout + 8 frame #9: 0x7fff944d61fc libdispatch.dylib`dispatch_once_f + 79 frame #10: 0x7fff987ced64 libsystem_info.dylib`si_module_static_search + 42 frame #11: 0x7fff987cec65 libsystem_info.dylib`si_module_with_name + 60 frame #12: 0x7fff987d0cf2 libsystem_inf
Re: [HACKERS] orangutan seizes up during isolation-check
Noah Misch writes: > Buildfarm member orangutan has failed chronically on both of the branches for > which it still reports, HEAD and REL9_1_STABLE, for over two years. The > postmaster appears to jam during isolation-check. Dave, orangutan currently > has one such jammed postmaster for each branch. Could you gather some > information about the running processes? What's particularly odd is that orangutan seems to be running an only slightly out-of-date OS X release, which is hardly an unusual configuration. My own laptop gets through isolation-check just fine. Seems like there must be something nonstandard about orangutan's software ... but what? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
