You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bojan Smojver <bo...@rexursive.com> on 2007/05/10 04:46:12 UTC

[PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

The problem is that the current 2.2.x code calls dbd_setup() only for
global server, therefore causing all other VHs to have things
uninitialised. If DBDPersist is On and dbd_setup_lock() is attempted,
mutex doesn't exist (it was never set up), so this fails. This patch
should fix all that (or so I hope :-).

Obviously, against 2.2.x branch.

-- 
Bojan

Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Bojan Smojver wrote:

>>    I apologize for joining this thread a little late.  I know it's
>> more complicated, but I'm inclined to suggest trying to bring the more
>> comprehensive trunk fixes into 2.2.x.
> 
> Just a ping on the status of this backport...

   The proposals are in the 2.2.x STATUS file, but no votes yet
(except mine).  I know the patches are a little daunting -- normally
I wouldn't propose such large changes as backports -- but in this
case I felt it was the best option, since the key "pools" and "groups"
changes are simpler to understand and implement if you've got the
other bits of tidying in place first.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2007-05-10 at 10:02 -0700, Chris Darroch wrote:

>    I apologize for joining this thread a little late.  I know it's
> more complicated, but I'm inclined to suggest trying to bring the more
> comprehensive trunk fixes into 2.2.x.

Just a ping on the status of this backport...

-- 
Bojan


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Bojan Smojver wrote:

> If mod_dbd.c from trunk works in 2.2.x, we should just have that
> instead. No need to carry two different things if the new stuff is
> backward compatible.

   If you need to, you can just drop the mod_dbd.c from trunk into
2.2.x; we do that and it works fine.  The main problem is simply that
trunk has had a series of substantial patches, all of which can all be
backported, but need some "round tuits", as Nick would say.

> PS. I was just responding to a privately asked question about problems
> with mod_dbd.c in 2.2.x and decided to send the resulting patches to the
> list, in case those problems were not already addressed.

   No problem ... I should have made up the backport proposal ages
ago, but have been buried in work until recently, and am still
digging my way out.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2007-05-10 at 10:02 -0700, Chris Darroch wrote:

>    I apologize for joining this thread a little late.  I know it's
> more complicated, but I'm inclined to suggest trying to bring the more
> comprehensive trunk fixes into 2.2.x.

If mod_dbd.c from trunk works in 2.2.x, we should just have that
instead. No need to carry two different things if the new stuff is
backward compatible.

PS. I was just responding to a privately asked question about problems
with mod_dbd.c in 2.2.x and decided to send the resulting patches to the
list, in case those problems were not already addressed.

-- 
Bojan


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Nick Kew wrote:

> I was wondering about that, but reluctant to propose a backport
> from trunk without doing some more research.  If you want to make it
> a backport proposal, I'll try and get my brain around it (and one or
> two related issues) in the morning.

   The main thing I'd point to is this long explanation of what the
issues I found were:

http://marc.info/?l=apache-httpd-dev&m=116742014418304&w=2

   If you follow the links at the bottom of this page, it maps my
original patches to what eventually got committed to trunk, along
with the comments in SVN:

http://people.apache.org/~chrisd/patches/mod_dbd_pools_groups/


> Anyone here using trunk mod_dbd operationally and have anything
> to report?
 
   We just drop the mod_dbd.c from trunk into 2.2.x and it works fine.
I did some really extensive testing of it beforehand, checking
all the various combinations of options, and I think it's pretty solid.

   IIRC, r491884 contains a number of fixes for old problems with
somewhat unusual combinations of things, like the use of ap_dbd_acquire()
on a non-threaded platform.  I tried to check all the combinations of
the following:

- with and without threads
- with and without persistent connections
- ap_dbd_[c]acquire(), with release during request or connection cleanup
- ap_dbd_open(), with explicit release using ap_dbd_close()
- failure during setup when creating persistent connection pools

   That's off the top of my head, so I may have missed a few things;
it's been a while since I thought about it.

   I'll try to start making up backports but it won't be tomorrow,
for sure.  :-/

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Nick Kew <ni...@webthing.com>.
On Thu, 10 May 2007 10:02:12 -0700
Chris Darroch <ch...@pearsoncmg.com> wrote:

> Nick Kew wrote:
> 
> > Thanks.  I've just reviewed both patches, and added them as an
> > attachment to PR#42327 and a proposal in STATUS.
> 
>    I apologize for joining this thread a little late.  I know it's
> more complicated, but I'm inclined to suggest trying to bring the more
> comprehensive trunk fixes into 2.2.x.
> 
>    First, can I suggest that PR #42327 be marked as a duplicate of
> PR #41302?  For many details, see this comment in particular, and its
> various references:
> 
> http://issues.apache.org/bugzilla/show_bug.cgi?id=41302#c1
> 
>    Trunk also includes a large number of other fixes, including those
> for PR #39985.  Both PR #39985 and PR #41302 have comments from
> their respective reporters indicating that the trunk version resolves
> their problems, and I think all three PRs could be closed as soon
> as backports are committed to 2.2.x.

I was wondering about that, but reluctant to propose a backport
from trunk without doing some more research.  If you want to make it
a backport proposal, I'll try and get my brain around it (and one or
two related issues) in the morning.

Anyone here using trunk mod_dbd operationally and have anything
to report?

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Nick Kew wrote:

> Thanks.  I've just reviewed both patches, and added them as an
> attachment to PR#42327 and a proposal in STATUS.

   I apologize for joining this thread a little late.  I know it's
more complicated, but I'm inclined to suggest trying to bring the more
comprehensive trunk fixes into 2.2.x.

   First, can I suggest that PR #42327 be marked as a duplicate of
PR #41302?  For many details, see this comment in particular, and its
various references:

http://issues.apache.org/bugzilla/show_bug.cgi?id=41302#c1

   Trunk also includes a large number of other fixes, including those
for PR #39985.  Both PR #39985 and PR #41302 have comments from
their respective reporters indicating that the trunk version resolves
their problems, and I think all three PRs could be closed as soon
as backports are committed to 2.2.x.

   Sorry again for the lack of time -- things should ease up for me
in a month or two.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c

Posted by Nick Kew <ni...@webthing.com>.
On Thu, 10 May 2007 12:46:12 +1000
Bojan Smojver <bo...@rexursive.com> wrote:

> The problem is that the current 2.2.x code calls dbd_setup() only for
> global server, therefore causing all other VHs to have things
> uninitialised. If DBDPersist is On and dbd_setup_lock() is attempted,
> mutex doesn't exist (it was never set up), so this fails. This patch
> should fix all that (or so I hope :-).
> 
> Obviously, against 2.2.x branch.

Thanks.  I've just reviewed both patches, and added them as an
attacment to PR#42327 and a proposal in STATUS.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex inmod_dbd.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2007-05-10 at 17:01 +1000, Bojan Smojver wrote:

> I don't think this applies to trunk. The trunk uses configuration groups
> and then applies dbd_setup to all of them.

Looks like r503931 was where those configuration groups were introduced.
This was the text associated with the commit:

---------------------------------
Introduce configuration groups to allow inheritance by virtual hosts of
database configurations from the main server.  The post_config hook function
determines the minimal set of distinct configurations necessary so that
database connection pools are shared between virtual hosts whenever possible.

The SQL statements which should be prepared for each database connection
are now stored in a hash for each virtual host during the configuration
phase, and these hashes are merged in the normal manner using
apr_hash_overlay() with that of the main server.  This allows for statements
to be de-registered by DBDPrepareSQL, if desired.  The post_config hook
function then compares the statements registered for each virtual host
when determining if a separate configuration group is required.  The
changes in r424798, r432560, r432562, and r466641, which still have problems
with configuration inheritance, are therefore no longer necessary.
---------------------------------

-- 
Bojan


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex inmod_dbd.c

Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2007-05-10 at 08:20 +0200, Plüm, Rüdiger, VF-Group wrote:

> As far as I understand your problem description the bug is only in 2.2.x and not in
> trunk. So could you please give a pointer to the revision(s) in trunk that fixed this?
> This can be considered for backport then. If this is not fixed in trunk yet, please
> sent a patch that applies to trunk.

I don't think this applies to trunk. The trunk uses configuration groups
and then applies dbd_setup to all of them.

So, this is a pure bug fix for 2.2.x branch. Ditto other patch I sent to
the list today.

-- 
Bojan


Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex inmod_dbd.c

Posted by Plüm, Rüdiger, VF-Group <ru...@vodafone.com>.

> -----Ursprüngliche Nachricht-----
> Von: Bojan Smojver 
> Gesendet: Donnerstag, 10. Mai 2007 04:46
> An: Apache Dev List
> Betreff: [PATCH]: Call dbd_setup() for all virtual hosts or 
> create mutex inmod_dbd.c
> 
> 
> The problem is that the current 2.2.x code calls dbd_setup() only for
> global server, therefore causing all other VHs to have things
> uninitialised. If DBDPersist is On and dbd_setup_lock() is attempted,
> mutex doesn't exist (it was never set up), so this fails. This patch
> should fix all that (or so I hope :-).
> 
> Obviously, against 2.2.x branch.

As far as I understand your problem description the bug is only in 2.2.x and not in
trunk. So could you please give a pointer to the revision(s) in trunk that fixed this?
This can be considered for backport then. If this is not fixed in trunk yet, please
sent a patch that applies to trunk.

Regards

Rüdiger