Re: [PATCH]: Call dbd_setup() for all virtual hosts or create mutex in mod_dbd.c
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
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
On Thu, 10 May 2007 12:46:12 +1000 Bojan Smojver [EMAIL PROTECTED] 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 in mod_dbd.c
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
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
On Thu, 10 May 2007 10:02:12 -0700 Chris Darroch [EMAIL PROTECTED] 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
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
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-devm=116742014418304w=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
[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
Index: modules/database/mod_dbd.c
===
--- modules/database/mod_dbd.c (revision 536730)
+++ modules/database/mod_dbd.c (working copy)
@@ -358,35 +358,42 @@
}
static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s)
{
-svr_cfg *svr = ap_get_module_config(s-module_config, dbd_module);
-apr_status_t rv;
+server_rec *sp;
+svr_cfg *svr;
+apr_status_t rv = APR_SUCCESS;
-/* dbd_setup in 2.2.3 and under was causing spurious error messages
- * when dbd isn't configured. We can stop that with a quick check here
- * together with a similar check in ap_dbd_open (where being
- * unconfigured is a genuine error that must be reported).
- */
-if (svr-name == no_dbdriver) {
-return APR_SUCCESS;
-}
+for (sp = s; sp; sp = sp-next) {
+svr = ap_get_module_config(sp-module_config, dbd_module);
-if (!svr-persist) {
-return APR_SUCCESS;
-}
+/* dbd_setup in 2.2.3 and under was causing spurious error messages
+ * when dbd isn't configured. We can stop that with a quick check
+ * here together with a similar check in ap_dbd_open (where being
+ * unconfigured is a genuine error that must be reported).
+ */
+if (svr-name == no_dbdriver) {
+continue;
+}
-rv = dbd_setup(pool, svr);
-if (rv == APR_SUCCESS) {
-return rv;
+if (!svr-persist) {
+continue;
+}
+
+rv = dbd_setup(pool, svr);
+if (rv == APR_SUCCESS) {
+continue;
+}
+
+/* we failed, so create a mutex so that subsequent competing callers
+ * to ap_dbd_open can serialize themselves while they retry
+ */
+rv = apr_thread_mutex_create(svr-mutex,
+ APR_THREAD_MUTEX_DEFAULT, pool);
+if (rv != APR_SUCCESS) {
+ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
+ DBD: Failed to create thread mutex);
+}
}
-/* we failed, so create a mutex so that subsequent competing callers
- * to ap_dbd_open can serialize themselves while they retry
- */
-rv = apr_thread_mutex_create(svr-mutex, APR_THREAD_MUTEX_DEFAULT, pool);
-if (rv != APR_SUCCESS) {
-ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
- DBD: Failed to create thread mutex);
-}
return rv;
}
static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s)
