enable-thread-safety defaults?

Started by Magnus Haganderover 16 years ago20 messageshackers
Jump to latest
#1Magnus Hagander
magnus@hagander.net

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/

#2Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#1)
Re: enable-thread-safety defaults?

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?

#3Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#2)
Re: enable-thread-safety defaults?

2009/11/20 Peter Eisentraut <peter_e@gmx.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/

#4Bruce Momjian
bruce@momjian.us
In reply to: Magnus Hagander (#3)
Re: enable-thread-safety defaults?

On Fri, Nov 20, 2009 at 7:39 AM, Magnus Hagander <magnus@hagander.net> wrote:

2009/11/20 Peter Eisentraut <peter_e@gmx.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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#3)
Re: enable-thread-safety defaults?

On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:

2009/11/20 Peter Eisentraut <peter_e@gmx.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.

#6Greg Smith
gsmith@gregsmith.com
In reply to: Peter Eisentraut (#5)
Re: enable-thread-safety defaults?

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@2ndQuadrant.com www.2ndQuadrant.com

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#5)
Re: enable-thread-safety defaults?

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

#8Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#5)
Re: enable-thread-safety defaults?

2009/11/20 Peter Eisentraut <peter_e@gmx.net>:

On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:

2009/11/20 Peter Eisentraut <peter_e@gmx.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/

#9Magnus Hagander
magnus@hagander.net
In reply to: Magnus Hagander (#8)
Re: enable-thread-safety defaults?

On Sat, Nov 21, 2009 at 08:29, Magnus Hagander <magnus@hagander.net> wrote:

2009/11/20 Peter Eisentraut <peter_e@gmx.net>:

On fre, 2009-11-20 at 08:39 +0100, Magnus Hagander wrote:

2009/11/20 Peter Eisentraut <peter_e@gmx.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/

Attachments:

configure.difftext/x-diff; charset=US-ASCII; name=configure.diffDownload+0-5
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#9)
Re: enable-thread-safety defaults?

Magnus Hagander <magnus@hagander.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

#11Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#10)
Re: enable-thread-safety defaults?

2009/11/24 Tom Lane <tgl@sss.pgh.pa.us>:

Magnus Hagander <magnus@hagander.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/

#12Bruce Momjian
bruce@momjian.us
In reply to: Magnus Hagander (#11)
Re: enable-thread-safety defaults?

Magnus Hagander wrote:

2009/11/24 Tom Lane <tgl@sss.pgh.pa.us>:

Magnus Hagander <magnus@hagander.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/pgpatches/threadtext/x-diffDownload+129-207
#13Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#12)
Re: enable-thread-safety defaults?

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.

#14Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#13)
Re: enable-thread-safety defaults?

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/pgpatches/threadtext/x-diffDownload+144-209
#15Magnus Hagander
magnus@hagander.net
In reply to: Bruce Momjian (#14)
Re: enable-thread-safety defaults?

2009/12/1 Bruce Momjian <bruce@momjian.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/

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#15)
Re: enable-thread-safety defaults?

Magnus Hagander <magnus@hagander.net> writes:

2009/12/1 Bruce Momjian <bruce@momjian.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

#17Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#16)
Re: enable-thread-safety defaults?

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

2009/12/1 Bruce Momjian <bruce@momjian.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#18Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#17)
Re: enable-thread-safety defaults?

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#19Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#17)
Re: enable-thread-safety defaults?

On tis, 2009-12-01 at 18:03 -0500, Bruce Momjian wrote:

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

2009/12/1 Bruce Momjian <bruce@momjian.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.

#20Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#19)
Re: enable-thread-safety defaults?

Peter Eisentraut wrote:

On tis, 2009-12-01 at 18:03 -0500, Bruce Momjian wrote:

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

2009/12/1 Bruce Momjian <bruce@momjian.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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