Re: enable-thread-safety defaults?

Is there any actual reason why we are building without thread safety
by default on most platforms? Seems I get asked that every time
somebody forgets to add a "--enable-thread-safety". Wouldn't it be
more logical to have that be the default, and provide
"--disable-thread-safety" if there are platforms that still don't
support it?

AFAIK pretty much all binary packages will do it by default, but it's
easy to forget when building from source....

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
> Is there any actual reason why we are building without thread safety
> by default on most platforms?

Consistent defaults on all platforms?

2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
>> Is there any actual reason why we are building without thread safety
>> by default on most platforms?
>
> Consistent defaults on all platforms?

So why do we have largefile enabled by default? And zlib? And readline?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Nov 20, 2009 at 7:39 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> 2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
>> On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
>>> Is there any actual reason why we are building without thread safety
>>> by default on most platforms?
>>
>> Consistent defaults on all platforms?
>
> So why do we have largefile enabled by default? And zlib? And readline?

Well those things are user interface options. They don't change the libpq api.

However given that distributions set this option I don't see much
point in worrying about the api being inconsistent. I agree that
having libpq being thread-safe would be a sensible default.

--
greg

On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:
> 2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> > On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
> >> Is there any actual reason why we are building without thread safety
> >> by default on most platforms?
> >
> > Consistent defaults on all platforms?
>
> So why do we have largefile enabled by default? And zlib? And readline?

Let me be more verbose: I would assume that we want the configure
defaults to be the same on all platforms. We fail by default, for
example, if zlib and readline are not there, but you can turn them off
explicitly. If we turn thread-safety on by default, we will/should fail
if thread-safety is not supported, requiring the user to turn it off
explicitly. If enough platforms don't support thread-safety, this could
become annoying.

I don't have a good overview over how many platforms would be affected,
and I could in general support changing the default, but I'm just laying
down one possible constraint.

Peter Eisentraut wrote:
> I don't have a good overview over how many platforms would be affected
>
The anniversary of this thread is a few days early:
http://archives.postgresql.org/message-id/492EA404.5080806@esilo.com

--
Greg Smith 2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com

Peter Eisentraut wrote:
>
> Let me be more verbose: I would assume that we want the configure
> defaults to be the same on all platforms. We fail by default, for
> example, if zlib and readline are not there, but you can turn them off
> explicitly. If we turn thread-safety on by default, we will/should fail
> if thread-safety is not supported, requiring the user to turn it off
> explicitly. If enough platforms don't support thread-safety, this could
> become annoying.
>
> I don't have a good overview over how many platforms would be affected,
> and I could in general support changing the default, but I'm just laying
> down one possible constraint.
>
>

Well, if we turn it on by default maybe the buildfarm will tell us who
the major culprits are :-)

cheers

andrew

2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:
>> 2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
>> > On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
>> >> Is there any actual reason why we are building without thread safety
>> >> by default on most platforms?
>> >
>> > Consistent defaults on all platforms?
>>
>> So why do we have largefile enabled by default? And zlib? And readline?
>
> Let me be more verbose:  I would assume that we want the configure
> defaults to be the same on all platforms.  We fail by default, for
> example, if zlib and readline are not there, but you can turn them off
> explicitly.  If we turn thread-safety on by default, we will/should fail
> if thread-safety is not supported, requiring the user to turn it off
> explicitly.

Yes, of course. Silently turning it off would be a really really bad idea.

> If enough platforms don't support thread-safety, this could
> become annoying.

Agreed.

> I don't have a good overview over how many platforms would be affected,
> and I could in general support changing the default, but I'm just laying
> down one possible constraint.

Well, the buildfarm would tell us that, no? :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sat, Nov 21, 2009 at 08:29, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> 2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
>> On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:
>>> 2009/11/20 Peter Eisentraut <peter_e(at)gmx(dot)net>:
>>> > On fre, 2009-11-20 at 02:41 +0100, Magnus Hagander wrote:
>>> >> Is there any actual reason why we are building without thread safety
>>> >> by default on most platforms?
>>> >
>>> > Consistent defaults on all platforms?
>>>
>>> So why do we have largefile enabled by default? And zlib? And readline?
>>
>> Let me be more verbose:  I would assume that we want the configure
>> defaults to be the same on all platforms.  We fail by default, for
>> example, if zlib and readline are not there, but you can turn them off
>> explicitly.  If we turn thread-safety on by default, we will/should fail
>> if thread-safety is not supported, requiring the user to turn it off
>> explicitly.
>
> Yes, of course. Silently turning it off would be a really really bad idea.
>
>> If enough platforms don't support thread-safety, this could
>> become annoying.
>
> Agreed.
>
>
>> I don't have a good overview over how many platforms would be affected,
>> and I could in general support changing the default, but I'm just laying
>> down one possible constraint.
>
> Well, the buildfarm would tell us that, no? :)

ISTM that it should be as simple as the attached patch. Seems to work
for me :-) But I'm no autoconf guru, so maybe I missed something?
Comments? If not, how about we put this on HEAD and let the buildfarm
tell us how bad an idea it was?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> ISTM that it should be as simple as the attached patch. Seems to work
> for me :-) But I'm no autoconf guru, so maybe I missed something?

This patch sort of begs the question "what about enable-thread-safety-force?"
That looks even more like a wart now than it did before.

regards, tom lane

2009/11/24 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> ISTM that it should be as simple as the attached patch. Seems to work
>> for me :-) But I'm no autoconf guru, so maybe I missed something?
>
> This patch sort of begs the question "what about enable-thread-safety-force?"
> That looks even more like a wart now than it did before.

Agreed. But how about we try it piece-by-piece, which is we start with
this to see if it actually hits any of our bf platforms?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander wrote:
> 2009/11/24 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> > Magnus Hagander <magnus(at)hagander(dot)net> writes:
> >> ISTM that it should be as simple as the attached patch. Seems to work
> >> for me :-) But I'm no autoconf guru, so maybe I missed something?
> >
> > This patch sort of begs the question "what about enable-thread-safety-force?"
> > That looks even more like a wart now than it did before.
>
> Agreed. But how about we try it piece-by-piece, which is we start with
> this to see if it actually hits any of our bf platforms?

Attached is a complete patch to enable threading of client libraries by
default --- I think it is time (threading was added to PG 7.4 in 2003).
I think we can guarantee that this will turn some build farm members
red. How do we pass --disable-thread-safety to those hosts?

The patch also removes --enable-thread-safety-force, which was added in
2004 for a platform that didn't have a thread-safe getpwuid():

http://archives.postgresql.org/pgsql-hackers/2004-07/msg00485.php

I think we can just tell people they have to upgrade their operating
systems if they want threading on those old platforms (or wait for
complaints).

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On mån, 2009-11-30 at 12:21 -0500, Bruce Momjian wrote:
> ! for thread safety; use --disable-thread-safety to disable
> threading.])

--disable-thread-safety does not disable threading, it disables thread
safety.

Peter Eisentraut wrote:
> On m?n, 2009-11-30 at 12:21 -0500, Bruce Momjian wrote:
> > ! for thread safety; use --disable-thread-safety to disable
> > threading.])
>
> --disable-thread-safety does not disable threading, it disables thread
> safety.

Good point! Patch updated and attached.

What are we going to do for build farm members who don't support
threading? Is someone going to manually update their configure flags?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

2009/12/1 Bruce Momjian <bruce(at)momjian(dot)us>:
> Peter Eisentraut wrote:
>> On m?n, 2009-11-30 at 12:21 -0500, Bruce Momjian wrote:
>> > ! for thread safety;  use --disable-thread-safety to disable
>> > threading.])
>>
>> --disable-thread-safety does not disable threading, it disables thread
>> safety.
>
> Good point!  Patch updated and attached.
>
> What are we going to do for build farm members who don't support
> threading?  Is someone going to manually update their configure flags?

Yeah, I think so.

Unless there's a whole lot of them, in which case we revert the patch.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> 2009/12/1 Bruce Momjian <bruce(at)momjian(dot)us>:
>> What are we going to do for build farm members who don't support
>> threading? Is someone going to manually update their configure flags?

> Yeah, I think so.

> Unless there's a whole lot of them, in which case we revert the patch.

It would seem like we ought to try the one-liner patch Magnus proposed
(ie flip the default) and see what the effects are, before we go with
the much larger patch Bruce wrote.

regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > 2009/12/1 Bruce Momjian <bruce(at)momjian(dot)us>:
> >> What are we going to do for build farm members who don't support
> >> threading? Is someone going to manually update their configure flags?
>
> > Yeah, I think so.
>
> > Unless there's a whole lot of them, in which case we revert the patch.
>
> It would seem like we ought to try the one-liner patch Magnus proposed
> (ie flip the default) and see what the effects are, before we go with
> the much larger patch Bruce wrote.

OK, done --- let the breakage begin. (I will be monitoring the build
farm and will work with Andrew Dunstan on any issues.)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> > It would seem like we ought to try the one-liner patch Magnus proposed
> > (ie flip the default) and see what the effects are, before we go with
> > the much larger patch Bruce wrote.
>
> OK, done --- let the breakage begin. (I will be monitoring the build
> farm and will work with Andrew Dunstan on any issues.)

OK, only Unixware and OpenBSD went red on the buildfarm with threading
enabled, so I have applied the more complete patch to enable thread
safety on clients by default, e.g. doc changes.

Andrew Dunstan is going to contact those build farm members so they use
--disable-thread-safety. He has also agreed to update the buildfarm to
detect this new option behavior.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On tis, 2009-12-01 at 18:03 -0500, Bruce Momjian wrote:
> Tom Lane wrote:
> > Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > > 2009/12/1 Bruce Momjian <bruce(at)momjian(dot)us>:
> > >> What are we going to do for build farm members who don't support
> > >> threading? Is someone going to manually update their configure flags?
> >
> > > Yeah, I think so.
> >
> > > Unless there's a whole lot of them, in which case we revert the patch.
> >
> > It would seem like we ought to try the one-liner patch Magnus proposed
> > (ie flip the default) and see what the effects are, before we go with
> > the much larger patch Bruce wrote.
>
> OK, done --- let the breakage begin. (I will be monitoring the build
> farm and will work with Andrew Dunstan on any issues.)

You have removed all the configure code that defined
ENABLE_THREAD_SAFETY, but there is still lots and lots of code that
checks for #ifdef ENABLE_THREAD_SAFETY. So this is is probably quite
broken at the moment.

Peter Eisentraut wrote:
> On tis, 2009-12-01 at 18:03 -0500, Bruce Momjian wrote:
> > Tom Lane wrote:
> > > Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > > > 2009/12/1 Bruce Momjian <bruce(at)momjian(dot)us>:
> > > >> What are we going to do for build farm members who don't support
> > > >> threading? Is someone going to manually update their configure flags?
> > >
> > > > Yeah, I think so.
> > >
> > > > Unless there's a whole lot of them, in which case we revert the patch.
> > >
> > > It would seem like we ought to try the one-liner patch Magnus proposed
> > > (ie flip the default) and see what the effects are, before we go with
> > > the much larger patch Bruce wrote.
> >
> > OK, done --- let the breakage begin. (I will be monitoring the build
> > farm and will work with Andrew Dunstan on any issues.)
>
> You have removed all the configure code that defined
> ENABLE_THREAD_SAFETY, but there is still lots and lots of code that
> checks for #ifdef ENABLE_THREAD_SAFETY. So this is is probably quite
> broken at the moment.

Yep, you have identified the bug, and I have applied the proper patch,
thanks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +