subject:"Re\: \[PATCHES\] ecpg thread\-safe descriptor"

On Tue, Oct 02, 2007 at 02:38:09PM +0900, ITAGAKI Takahiro wrote:
 Here is a patch to fix thread-safety of SQL DESCRIPTOR in ecpg.
 
 The global variable 'all_descriptors' is split into per-thread vars.
 There was another idea of splitting into per-connection vars, but
 I did not do that because there are allocating descriptors before
 connection and deallocating after disconnection in the regression test.
 
 The attached descriptor.pgc is a test case for the issue.
 It is not included into regression test of ecpg.

(haven't looked at the patch itself)


 If this change is acceptable to be applied to HEAD, I'll start backporting
 recent fixes (prepared statement, memory allocation and descriptor) to 8.2
 (and older versions if possible).

Are you sure we should backport this to 8.2 and earlier? It's fairly large
changes, and not necessarily entirely straightforward (for example, the
buildfarm is still partially broken from the earlier ones not being
portable enough). 

I think this can be considered a new feature and not just a plain bugfix,
so it should probably only go into HEAD.

//Magnus

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ecpg thread-safe descriptor

2007-10-02 Thread ITAGAKI Takahiro


Magnus Hagander [EMAIL PROTECTED] wrote:

 Are you sure we should backport this to 8.2 and earlier? It's fairly large
 changes, and not necessarily entirely straightforward (for example, the
 buildfarm is still partially broken from the earlier ones not being
 portable enough).

Almost portability issues come from Windows, but ecpg in 8.2 only support
thread-safety on pthread, not on Win32. So backporting is easier than
expected because we can neglect Windows.

I want thread-safe ecpg *NOW* and cannot wait for 8.3 release...
At any rate, I'll fix it at hand and report it if it works well.

BTW, do we need to distribute ecpg with server (or client library) ?
If it was a separate product, I think we will be able to maintain them
more flexiblly, just like libpqxx or JDBC.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 02:38:09PM +0900, ITAGAKI Takahiro wrote:
 Here is a patch to fix thread-safety of SQL DESCRIPTOR in ecpg.
 ...

Applied to CVS HEAD. Thanks.

Michael

-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 12:23:10PM +0200, Michael Meskes wrote:
 On Tue, Oct 02, 2007 at 06:45:12PM +0900, ITAGAKI Takahiro wrote:
  Almost portability issues come from Windows, but ecpg in 8.2 only support
  thread-safety on pthread, not on Win32. So backporting is easier than
  expected because we can neglect Windows.
 
 True, but it's still a pretty huge change. I'd prefer to only backport
 small fixes not serious rewrites.
 
  I want thread-safe ecpg *NOW* and cannot wait for 8.3 release...
 
 Which is a valid point for backporting your changes but NOT for
 including them into CVS. 

Yeah, that's all I object to of course. Not the actual backporting.


  BTW, do we need to distribute ecpg with server (or client library) ?
  If it was a separate product, I think we will be able to maintain them
  more flexiblly, just like libpqxx or JDBC.
 
 I do not agree at all with this statement. IMO it's absolutely necessary
 that ecpg's parser is in sync with the backend's parser. So essentially
 we would not gain any flexibility as we'd have to release at the same
 time anyway.

That's the reason I recall for keeping it in there as well.

Michael, any chance to consider the exports-from-libecpg issue I posted
about the other day?

//Magnus

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ecpg thread-safe descriptor

Magnus Hagander [EMAIL PROTECTED] writes:
 I think this can be considered a new feature and not just a plain bugfix,
 so it should probably only go into HEAD.

I am very nearly ready to demand that ecpg as a whole be reverted to its
state as of Saturday, with all the patches since then held for 8.4
development.  At no point since Sunday morning has ecpg been passing
buildfarm testing, and I see no activity indicating that it's about to
get fixed.  You guys are holding up beta release with work that by any
legitimate definition is new-feature development, and shouldn't be under
consideration at all at this point.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 10:39:40AM -0400, Tom Lane wrote:
 I am very nearly ready to demand that ecpg as a whole be reverted to its
 state as of Saturday, with all the patches since then held for 8.4
 development.  At no point since Sunday morning has ecpg been passing

If we say ecpg is not multithreaded I agree that these patches are
features, but if we see it as multithreaded the patches are bug fixes.

 buildfarm testing, and I see no activity indicating that it's about to

Magnus, correct me if I'm wrong, but it appears to me that the MSVC
build problem has existed for weeks or months but no one noticed so far,
right? It doesn't help to revert the patches if we still do not compile
cleanly on MSVC. Or is it only the DLLMAIN thing that's holding us up?
If so couldn't we just remove this function and call MSVC build not
multithreaded?

Also Takahiro (hope this is your first name :-)), could you please
explain to us why DllMain is needed at all? it isn't needed on
non-Windows systems it seems.

As far as fixing things you're right. We need to fix this asap.

 get fixed.  You guys are holding up beta release with work that by any
 legitimate definition is new-feature development, and shouldn't be under
 consideration at all at this point.

As I said, I considered this work bug fixes or else I wouldn't have
committed the patches at this time.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

2007-10-02 Thread Andrew Dunstan




Michael Meskes wrote:

It it appears to me that the MSVC
build problem has existed for weeks or months but no one noticed so far,
right? 



This is absolutely not true. We have been building ecpg successfully on 
MSVC all along, just not testing. See for example the make log at 
http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=red_batdt=2007-09-30%20043002stg=make 
and search for Project: libecpg


The builds only broke 2 or 3 days ago.

cheers

andrew

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 12:53:58PM -0400, Andrew Dunstan wrote:
 It it appears to me that the MSVC
 build problem has existed for weeks or months but no one noticed so far,
 right? 

 This is absolutely not true. We have been building ecpg successfully on 
 MSVC all along, just not testing. See for example the make log at 
 http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=red_batdt=2007-09-30%20043002stg=make
  
 and search for Project: libecpg

Sorry, my bad. Seems I got this mixed up. Once we enabled testing for
ecpg we had some problems to be sorted out, but right now only the
DllMain thing seems to be a problem.

Since I do not know whether we really need DllMain could anyone please
test whether ecpg builds correctly without it? We can then sort out how
to fix the problem later on. AFAIRC (and hopefully I'm right this time)
multithreading never worked under msvc so we might have to set
thread-safety to disabled on these systems for ecpg for the time being.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

Michael Meskes [EMAIL PROTECTED] writes:
 Sorry, my bad. Seems I got this mixed up. Once we enabled testing for
 ecpg we had some problems to be sorted out, but right now only the
 DllMain thing seems to be a problem.

 Since I do not know whether we really need DllMain could anyone please
 test whether ecpg builds correctly without it?

It looks to me like we should not need it, if we instead add code to
initialize the mutexes properly on Windows (a la the way it's done
in libpq).  I can prepare a patch, but I don't have the ability to
test it on Windows --- will anyone be around who can, in an hour or so?

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ecpg thread-safe descriptor

Michael Meskes wrote:
On Tue, Oct 02, 2007 at 12:53:58PM -0400, Andrew Dunstan wrote:
It it appears to me that the MSVC
build problem has existed for weeks or months but no one noticed so far,
right?
This is absolutely not true. We have been building ecpg successfully on
MSVC all along, just not testing. See for example the make log at
http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=red_batdt=2007-09-30%20043002stg=make

and search for Project: libecpg

Sorry, my bad. Seems I got this mixed up. Once we enabled testing for
ecpg we had some problems to be sorted out, but right now only the
DllMain thing seems to be a problem.

There are two problems:
1) DllMain breaks the build *completely*. This is the really urgent one.
This one broke 2 days ago or so.

2) Regression tests don't pass on MSVC anymore. This broke a while ago.
I was working on fixing this, but now that the build is broken there
isn't much I can do until that's fixed.

Since I do not know whether we really need DllMain could anyone please
test whether ecpg builds correctly without it? We can then sort out how
to fix the problem later on. AFAIRC (and hopefully I'm right this time)
multithreading never worked under msvc so we might have to set
thread-safety to disabled on these systems for ecpg for the time being.

We could move the initialization somewhere else, but it needs to be
somewhere.

Also, see
http://archives.postgresql.org/pgsql-hackers/2007-10/msg6.php for
another comment on this. This changes more than just the threading
stuff, but if it's something we want then this is a good time to do it.

//Magnus

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Re: [PATCHES] ecpg thread-safe descriptor

Tom Lane wrote:
 Michael Meskes [EMAIL PROTECTED] writes:
 Sorry, my bad. Seems I got this mixed up. Once we enabled testing for
 ecpg we had some problems to be sorted out, but right now only the
 DllMain thing seems to be a problem.
 
 Since I do not know whether we really need DllMain could anyone please
 test whether ecpg builds correctly without it?
 
 It looks to me like we should not need it, if we instead add code to
 initialize the mutexes properly on Windows (a la the way it's done
 in libpq).  I can prepare a patch, but I don't have the ability to
 test it on Windows --- will anyone be around who can, in an hour or so?

Should be doable. I can probably (but not certainly, so if someone else
is around that'd be nice) get close enough to remote a windows box to
test that a build passes, but remember that we can't really run the
regression tests on msvc right now :-(

//Magnus

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [PATCHES] ecpg thread-safe descriptor

I wrote:
 Michael Meskes [EMAIL PROTECTED] writes:
 Since I do not know whether we really need DllMain could anyone please
 test whether ecpg builds correctly without it?

 It looks to me like we should not need it, if we instead add code to
 initialize the mutexes properly on Windows (a la the way it's done
 in libpq).

Hm, I take that back --- the other reason it's there is as a kluge to
avoid supporting pthread_once().  This code all looks pretty darn hokey,
not to mention that it ought to be merged with libpq's pthread-win32.c,
but fixing it seems not a one-hour proposition.

 AFAIRC (and hopefully I'm right this time)
 multithreading never worked under msvc so we might have to set
 thread-safety to disabled on these systems for ecpg for the time being.

If it didn't work then this seems clearly new-feature-development.

I think Magnus' idea of providing an exports list would be worth trying,
if someone can try it promptly; failing that, we need to revert this
patch.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 08:23:21PM +0200, Magnus Hagander wrote:
 test that a build passes, but remember that we can't really run the
 regression tests on msvc right now :-(

But then we definitely need to fix this, regardless of the DllMain thing.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] ecpg thread-safe descriptor

Michael Meskes wrote:
 On Tue, Oct 02, 2007 at 08:23:21PM +0200, Magnus Hagander wrote:
 test that a build passes, but remember that we can't really run the
 regression tests on msvc right now :-(
 
 But then we definitely need to fix this, regardless of the DllMain thing.

Absolutely, but we need to fix the DllMain thing first.

//Magnus

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 02:37:40PM -0400, Tom Lane wrote:
  AFAIRC (and hopefully I'm right this time)
  multithreading never worked under msvc so we might have to set
  thread-safety to disabled on these systems for ecpg for the time being.
 
 If it didn't work then this seems clearly new-feature-development.

Well, ecpg was said to work with multithreading but didn't. However,
since the patch works on most archs if indeed fixes some bugs on those
archs.

 I think Magnus' idea of providing an exports list would be worth trying,
 if someone can try it promptly; failing that, we need to revert this
 patch.

Couldn't we just disable multithreading for Windows resp. MSVC? It
appears to me that the rest of the patch is working as advertized.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] ecpg thread-safe descriptor

On Tue, Oct 02, 2007 at 08:22:21PM +0200, Magnus Hagander wrote:
 There are two problems:
 1) DllMain breaks the build *completely*. This is the really urgent one.
 This one broke 2 days ago or so.
 
 2) Regression tests don't pass on MSVC anymore. This broke a while ago.
 I was working on fixing this, but now that the build is broken there
 isn't much I can do until that's fixed.

When did they break? I'm still confused. Did we run regression tests at
all before? Or did they never work and we just didn't notice because the
buildfarm didn't run them?

 Also, see
 http://archives.postgresql.org/pgsql-hackers/2007-10/msg6.php for
 another comment on this. This changes more than just the threading
 stuff, but if it's something we want then this is a good time to do it.

So, does an explicit export list help? If so I'm all for it. There is no
need to export all symbols. I always tried to keep the number of symbols
that get exported but are not needed low. So this will give the best
result possible.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL!

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Re: [PATCHES] ecpg thread-safe descriptor