Re: [PATCH] mod_dbd with more than one pool
Topic up! Issue in bugzilla: https://issues.apache.org/bugzilla/show_bug.cgi?id=45456 On Tue, Mar 31, 2009 at 2:08 PM, Kevac Marko wrote: > Nick, any news? > > On Fri, Mar 13, 2009 at 3:54 PM, Nick Kew wrote: >> Thanks. I'll test-drive today. > > -- > Marko Kevac > -- A. Because it breaks the logical sequence of discussion Q. Why is top posting bad?
Re: [PATCH] mod_dbd with more than one pool
Nick, any news? On Fri, Mar 13, 2009 at 3:54 PM, Nick Kew wrote: > Thanks. I'll test-drive today. -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
On Tue, Mar 24, 2009 at 10:03 PM, Ronald Park wrote: > A user might presume that the order of commands listed in the > config file would be preserved when the commands are run. By using a hash > table, you'll likely get a random order causing problems. For example, > perhaps the user needs to first load one table then choose a subset from > that first table to file another; if you run them in the reserve order, the > second table will likely be empty. :( Thank you. I will implement this. -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
Ronald Park wrote: I did have one minor nit to pick with the implementation of the DBDInitSQL command. A user might presume that the order of commands listed in the config file would be preserved when the commands are run. By using a hash table, you'll likely get a random order causing problems. For example, perhaps the user needs to first load one table then choose a subset from that first table to file another; if you run them in the reserve order, the second table will likely be empty. :( Thanks for the comment -- noted in PR #46827. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [PATCH] mod_dbd with more than one pool
Sorry for coming into this discussion late. I think this is an excellent set of patches. I had developed a similar enhancement to allow mod_dbd to connect with multiple databases at my last job but never had a chance to make it available here. Your version is far more clever and configurable. I did have one minor nit to pick with the implementation of the DBDInitSQL command. A user might presume that the order of commands listed in the config file would be preserved when the commands are run. By using a hash table, you'll likely get a random order causing problems. For example, perhaps the user needs to first load one table then choose a subset from that first table to file another; if you run them in the reserve order, the second table will likely be empty. :( If you choose not to change the implementation, then the documentation should make this clear. Thanks, Ron Principal Software Engineer for Hire http://www.linkedin.com/in/ronaldparkjr
Re: [PATCH] mod_dbd with more than one pool
On Fri, Mar 13, 2009 at 2:54 PM, Nick Kew wrote: > Thanks. I'll test-drive today. Succeeded? -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
On 13 Mar 2009, at 10:21, Kevac Marko wrote: https://issues.apache.org/bugzilla/show_bug.cgi?id=46827 Implemented. Patch added. Comments? Thanks. I'll test-drive today. -- Nick Kew
Re: [PATCH] mod_dbd with more than one pool
On Tue, Mar 10, 2009 at 10:45 PM, Nick Kew wrote: > Can I throw an alternative suggestion into the ring. > Instead of running dbd_init_sql_init at the end of > dbd_construct, run a hook there. Your function then > runs on that hook, but it enables other modules > to do their own thing too. > > I think it's a mistake in the current mod_dbd that > dbd_prepared_init isn't hooked, and having the hook > could've helped with some of the bugs we've seen. > While you're hacking it would seem like a good > opportunity to rectify all that! https://issues.apache.org/bugzilla/show_bug.cgi?id=46827 Implemented. Patch added. Comments? -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
Nick Kew wrote: Kevac Marko wrote: Ok, here is sql init statement only patch against trunk: https://issues.apache.org/bugzilla/show_bug.cgi?id=46827 Thanks for the patch! Can I throw an alternative suggestion into the ring. [ ... ] Thoughts? A further thought. I had in mind PR#45407 - mysql losing prepared statements on reconnect - and possible similar problems, when I suggested a hook there. Then the driver just needs to return an error status that says "you may need to run this hook now", and mod_dbd can do so. I see you're one of the people who contributed to the discussion on that bug. Did you bear it in mind in your new stuff? -- Nick Kew
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: Ok, here is sql init statement only patch against trunk: https://issues.apache.org/bugzilla/show_bug.cgi?id=46827 Thanks for the patch! Can I throw an alternative suggestion into the ring. Instead of running dbd_init_sql_init at the end of dbd_construct, run a hook there. Your function then runs on that hook, but it enables other modules to do their own thing too. I think it's a mistake in the current mod_dbd that dbd_prepared_init isn't hooked, and having the hook could've helped with some of the bugs we've seen. While you're hacking it would seem like a good opportunity to rectify all that! Thoughts? -- Nick Kew
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: It's great idea. But are you sure that it is good idea to change both main httpd.conf and VirtualHost for new DB connection to be added? These are separate things... It will introduce problems if, for example, httpd.conf is root writable and virtualhost-blabla.conf is user writable. I don't think anything now prevents you from defining DB connection configurations in the vhost which are private to it, which would suit your use case, I believe, so long as we retain that option. I can also imagine that an administrator might want to provide a "slate" of pre-rolled DB configurations from which vhost users would be encouraged to choose. That would help reduce a problem you might otherwise experience, if you had a large number of vhosts and everyone rolled their own DB configurations, where because each configuration was slightly different (e.g., different expiry times or what have you) you'd overwhelm the DB server with connections. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: Prepared statements are not executed, just parsed (simplified), so no, it is not important for initialization statement to be before prepared statements. Right; what I wondered was whether you needed to execute some sort of "magic" initialization statement which would then allow normal processing, including preparing regular statements, to proceed. From the bug report (thanks for that) I see your example is setting character sets with MySQL, which I take it doesn't affect preparing statements, but I'm not a MySQL expert. Perhaps other DBs in certain configurations might have initialization requirements that do affect preparing statements? Since I'm not an expert in all the different DBs and their possible configurations, I just wanted to open the subject for discussion. It also seems to me that it doesn't hurt to do the initializations first, just in case. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [PATCH] mod_dbd with more than one pool
On Thu, Mar 5, 2009 at 10:05 PM, Chris Darroch wrote: > One difference, I think -- again, after only a quick review -- is > that what I was trying to solve with the notion of a > container and then a DBDGroup directive was to minimize the number > of distinct configuration groups and make configuration as simple > as possible. You could put, say, two sets of directives > into the main server configuration. Then in a virtual host, you'd > just need to say "DBDGroup foo" to specify which one you wanted > to use there. It's great idea. But are you sure that it is good idea to change both main httpd.conf and VirtualHost for new DB connection to be added? These are separate things... It will introduce problems if, for example, httpd.conf is root writable and virtualhost-blabla.conf is user writable. > Your changes seem targeted at the issue of supporting > multiple connections within, say, a single virtual host. I'm not > 100% sure if that's the idea, so please correct me if I'm wrong. > Then callers who want a particular connection pass the DBD "pool" > name to ap_dbd_open_pool(), etc. Yes. We configure separate connection pools to separate DBses. MySQL, PostgreSQL, SQLite for example on different hosts and give them names. And user can chose connection from which connection pool he wants. > This seems like a good enhancement to me, certainly, and we've > seen some requests for it on the mailing list. If we're adding > such configuration containers, though, I personally would want to > ensure that we had a way to minimize the number of distinct > connection pools involved. > > It looks to me as though the logic in dbd_post_config() which > aims to do this, i.e., minimize the number of distinct DBD > configurations (when using the existing set of directives) is removed > in the patch. I may be missing something, but I think that's likely > to be necessary in the future as well, to support existing installations > and not require administrators to rewrite their configurations > after upgrading. I have tried to keep everything concerning group minimization from original mod_dbd. If not - it is bug. I don't really like the way that minimization was implemented, but i have tried to minimize changes to mod_dbd source. > Suppose you had many virtual hosts and each one needed one or > both of two types of DB connection. You'd want to be able to specify > the two types of connection in just two or > containers at the main server level, it seems to me. Then each > virtual host could refer back to the ones it required, if, for > instance, it was adding vhost-specific DBD authentication directives. But what if owner of blahblah.host.org needs third connection pool and he can't change httpd.conf? -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
On Fri, Mar 6, 2009 at 7:58 PM, Chris Darroch wrote: > One thought I had overnight is that you might, if you like, want > to tackle the init SQL patch first, which seems unrelated to the > multiple pools idea and significantly simpler, easier to review, > and probably easier to backport as well. Done. > Can you provide an example of how such initialization SQL queries > would be used? And, as a question, might it be necessary for such > queries to run before any other SQL queries are prepared? IIRC > in the patch they are run after the other queries are prepared, but > if they were truly required for connection initialization, they'd > need to be absolutely first, no? Prepared statements are not executed, just parsed (simplified), so no, it is not important for initialization statement to be before prepared statements. -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
Ok, here is sql init statement only patch against trunk: https://issues.apache.org/bugzilla/show_bug.cgi?id=46827 P.S. Thanks http://jukka.zitting.name/git/ for git mirrors. I am happy :-) P.P.S. Probably I should wait for sql init statement only patch inclusion before posting everything else? -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: I'm not experienced in commiting patches to open source projects, so I am very thankful for your comments. No problem -- thanks for pushing this patch along! One thought I had overnight is that you might, if you like, want to tackle the init SQL patch first, which seems unrelated to the multiple pools idea and significantly simpler, easier to review, and probably easier to backport as well. Can you provide an example of how such initialization SQL queries would be used? And, as a question, might it be necessary for such queries to run before any other SQL queries are prepared? IIRC in the patch they are run after the other queries are prepared, but if they were truly required for connection initialization, they'd need to be absolutely first, no? 80 columns is a little bit ancient requirement in the world of 22" LCDs, but ok, i'll fix that too. I mentioned this just because it's part of the httpd style guidelines, and because there's no need to reformat existing code which already meets the guideline. This is off-topic, but personally I like such a limit not only for the key reasons already mentioned by Brian (i.e., having two or more versions of the same code or some inter-related files next to each other), but also because I find it helps curtail poor design on my part. If I've got a deeply nested set of blocks and I'm hitting up against the right margin, that's a good hint I need to rethink what I'm doing and likely refactor it into some smaller functions. It also forces one to avoid names like foo_bar_baz_wadda_wadda_yippity_boom_de_boom() -- or in Java, fooBarBazWaddaWaddaYippityBoomDeBoom() -- and find something shorter and more expressive. And if I have a function with a large number of arguments, it makes me review whether they should really be part of a single data structure or object; that's been a good reminder to me over the years of Fred Brooks's old adage about thinking through one's data structures first. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: There are some bugs in non-threaded build. I am fixing them right now. Another quick request: is it possible to update the apr_dbd end user documentation to show the additional container directive, and to explain how the module should work? Regards, Graham -- smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] mod_dbd with more than one pool
There are some bugs in non-threaded build. I am fixing them right now. -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
On Mar 5, 2009, at 4:52 PM, Kevac Marko wrote: 80 columns is a little bit ancient requirement in the world of 22" LCDs, but ok, i'll fix that too. Not when you have 4 code branches side by side... Also, netbooks are pretty popular as well. Most of my coding is done on a 15" laptop screen. --Brian
Re: [PATCH] mod_dbd with more than one pool
Chris Darroch wrote: Over in mod_dbd.h, Nick's old copyright statement is removed. That's definitely something which, if needed, should be dealt with separately -- I'm not sure if it should be there or not, but it's a legal issue not a coding one. OK, guess it's best if I deal with that one. Fixed in r750620. -- Nick Kew
Re: [PATCH] mod_dbd with more than one pool
Chris, thank you. I am impressed. I'm not experienced in commiting patches to open source projects, so I am very thankful for your comments. I'll try to fix things up very soon. 80 columns is a little bit ancient requirement in the world of 22" LCDs, but ok, i'll fix that too. -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: Here I am again. The patch works well for us. Is there something else that I can do for now? I'm going to try to find some time to take a look -- thanks for the patch; it's good to see the idea being taken further than my initial notions. On an initial quick glance, there are a few things we'll need to do before we could apply this patch as-is (aside from any questions of functionality or testing). There are some whitespace and formatting changes in the patch which make it a little harder than necessary to comprehend what's going on. (This is something I'm guilty of as well; I know how tempting it is to fix whitespace while refactoring.) For example, ap_dbd_prepare()'s DBD_DECLARE_NONSTD definition in mod_dbd.c goes from two lines to one; the original was more in the httpd style. See http://httpd.apache.org/dev/styleguide.html for the gruesome details. The same is true of various other function definitions where the arguments spill past 80 columns. There's a switch statement in dbd_param_int() which has been reformatting away from the httpd style -- case statements should be aligned with the parent switch statement. As far as I can see there are no functional changes here, so this should be left as-is. Same thing in dbd_param(), I think. Some of the error messages will need a grammar check, e.g., "could not found dbd configuration for pool" should probably be "could not find DBD configuration for pool". The DEFAULT_POOL_COUNT macro seems to be unused; that could be removed. Over in mod_dbd.h, Nick's old copyright statement is removed. That's definitely something which, if needed, should be dealt with separately -- I'm not sure if it should be there or not, but it's a legal issue not a coding one. The other changes in mod_dbd.h look to be, at a functional level, just adding some declarations. However, there's a fair bit of reformatting going on as well, such as arguments being given names in existing functions (ap_dbd_cacquire(conn_rec *c) versus the old ap_dbd_cacquire(conn_rec*)). That's a nice change, but we should split that out as a separate reformatting patch before making any functional changes, or else leave it alone and just concentrate on the functional stuff. One option is a "jumbo" reformatting patch which precedes all the subsequent functional changes. As for the non-formatting changes, it would be great to see a patch with just those pared down to the minimal set of revisions necessary. From a quick review I get the general drift; there's a hash of DBD "pools" (not memory pools) for each server config and legacy DBD config directives, those outside a container, are attached to the DEFAULT_DBD_POOL_NAME. This would seem to correspond pretty nicely with the idea I envisioned back in late 2006; see the end of this email: http://marc.info/?l=apache-httpd-dev&m=116742014418304&w=2 One difference, I think -- again, after only a quick review -- is that what I was trying to solve with the notion of a container and then a DBDGroup directive was to minimize the number of distinct configuration groups and make configuration as simple as possible. You could put, say, two sets of directives into the main server configuration. Then in a virtual host, you'd just need to say "DBDGroup foo" to specify which one you wanted to use there. Your changes seem targeted at the issue of supporting multiple connections within, say, a single virtual host. I'm not 100% sure if that's the idea, so please correct me if I'm wrong. Then callers who want a particular connection pass the DBD "pool" name to ap_dbd_open_pool(), etc. This seems like a good enhancement to me, certainly, and we've seen some requests for it on the mailing list. If we're adding such configuration containers, though, I personally would want to ensure that we had a way to minimize the number of distinct connection pools involved. It looks to me as though the logic in dbd_post_config() which aims to do this, i.e., minimize the number of distinct DBD configurations (when using the existing set of directives) is removed in the patch. I may be missing something, but I think that's likely to be necessary in the future as well, to support existing installations and not require administrators to rewrite their configurations after upgrading. Suppose you had many virtual hosts and each one needed one or both of two types of DB connection. You'd want to be able to specify the two types of connection in just two or containers at the main server level, it seems to me. Then each virtual host could refer back to the ones it required, if, for instance, it was adding vhost-specific DBD authentication directives. The existing mod_dbd will add such authentication directives -- which from mod_dbd's point of view are additional queries to prepare after connection -- without requiring a whole new DBD connection. If an administrator might want to apply authentication dire
Re: [PATCH] mod_dbd with more than one pool
On Thu, Feb 12, 2009 at 12:56 AM, Nick Kew wrote:
> Sounds OK in principle for trunk. If you want to post a patch
> against trunk, I'll try and find the time to review it.
Here I am again.
The patch works well for us.
Is there something else that I can do for now?
--
Marko Kevac
diff -u trunk/mod_dbd.c mine/mod_dbd.c
--- trunk/mod_dbd.c 2009-03-05 15:19:18.0 +0300
+++ mine/mod_dbd.c 2009-03-05 15:19:44.0 +0300
@@ -20,6 +20,8 @@
* http://apache.webthing.com/database/
*/
+#define CORE_PRIVATE
+
#include "apr_reslist.h"
#include "apr_strings.h"
#include "apr_hash.h"
@@ -46,8 +48,13 @@
#define NMAX_SET 0x4
#define EXPTIME_SET 0x8
+#define DEFAULT_POOL_COUNT 2
+
+#define DBD_DEFAULT_POOL_NAME "dbd_default_pool"
+
typedef struct {
server_rec *server;
+const char *pool_name;
const char *name;
const char *params;
int persist;
@@ -59,6 +66,7 @@
int set;
#endif
apr_hash_t *queries;
+apr_hash_t *init_queries;
} dbd_cfg_t;
typedef struct dbd_group_t dbd_group_t;
@@ -77,8 +85,8 @@
};
typedef struct {
-dbd_cfg_t *cfg;
-dbd_group_t *group;
+apr_hash_t *cfgs;
+apr_hash_t *groups;
} svr_cfg;
typedef enum { cmd_name, cmd_params, cmd_persist,
@@ -102,79 +110,145 @@
static void *create_dbd_config(apr_pool_t *pool, server_rec *s)
{
svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg));
-dbd_cfg_t *cfg = svr->cfg = apr_pcalloc(pool, sizeof(dbd_cfg_t));
-cfg->server = s;
-cfg->name = no_dbdriver; /* to generate meaningful error messages */
-cfg->params = ""; /* don't risk segfault on misconfiguration */
-cfg->persist = -1;
-#if APR_HAS_THREADS
-cfg->nmin = DEFAULT_NMIN;
-cfg->nkeep = DEFAULT_NKEEP;
-cfg->nmax = DEFAULT_NMAX;
-cfg->exptime = DEFAULT_EXPTIME;
-#endif
-cfg->queries = apr_hash_make(pool);
+svr->cfgs = apr_hash_make(pool);
+svr->groups = apr_hash_make(pool);
return svr;
}
-static void *merge_dbd_config(apr_pool_t *pool, void *basev, void *addv)
+DBD_DECLARE_NONSTD(void) ap_dbd_sql_init(server_rec *s, const char *pool_name,
+const char *query, const char *label)
{
-dbd_cfg_t *base = ((svr_cfg*) basev)->cfg;
-dbd_cfg_t *add = ((svr_cfg*) addv)->cfg;
-svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg));
-dbd_cfg_t *new = svr->cfg = apr_pcalloc(pool, sizeof(dbd_cfg_t));
+svr_cfg *svr;
+dbd_cfg_t *cfg = NULL;
-new->server = add->server;
-new->name = (add->name != no_dbdriver) ? add->name : base->name;
-new->params = strcmp(add->params, "") ? add->params : base->params;
-new->persist = (add->persist != -1) ? add->persist : base->persist;
-#if APR_HAS_THREADS
-new->nmin = (add->set&NMIN_SET) ? add->nmin : base->nmin;
-new->nkeep = (add->set&NKEEP_SET) ? add->nkeep : base->nkeep;
-new->nmax = (add->set&NMAX_SET) ? add->nmax : base->nmax;
-new->exptime = (add->set&EXPTIME_SET) ? add->exptime : base->exptime;
-#endif
-new->queries = apr_hash_overlay(pool, add->queries, base->queries);
+svr = ap_get_module_config(s->module_config, &dbd_module);
+if (!svr) {
+ /* some modules may call from within config directive handlers, and
+ * if these are called in a server context that contains no mod_dbd
+ * config directives, then we have to create our own server config
+ */
+ svr = create_dbd_config(config_pool, s);
+ ap_set_module_config(s->module_config, &dbd_module, svr);
+}
-return svr;
+cfg = apr_hash_get(svr->cfgs, pool_name, APR_HASH_KEY_STRING);
+if (NULL == cfg) {
+ap_log_error(APLOG_MARK, APLOG_CRIT, 0, s,
+"could not found dbd configuration for pool %s", pool_name);
+return;
+}
+
+if (apr_hash_get(cfg->init_queries, label, APR_HASH_KEY_STRING)
+&& strcmp(query, "")) {
+ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
+"conflicting SQL statements with label %s", label);
+}
+
+apr_hash_set(cfg->init_queries, label, APR_HASH_KEY_STRING, query);
+}
+
+static const char *dbd_pool(cmd_parms *cmd, void *mconfig, const char *arg)
+{
+const char *errmsg = NULL;
+const char *endp = ap_strrchr_c(arg, '>');
+
+dbd_cfg_t *conf;
+
+ap_conf_vector_t *new_dir_conf = ap_create_per_dir_config(cmd->pool);
+
+const char *err = ap_check_cmd_context(cmd,
+ NOT_IN_DIR_LOC_FILE|NOT_IN_LIMIT);
+
+if (err != NULL) {
+return err;
+}
+
+if (endp == NULL) {
+return apr_pstrcat(cmd->pool, cmd->cmd->name,
+ "> directive missing closing '>'", NULL);
+}
+
+arg = apr_pstrndup(cmd->pool, arg, endp - arg);
+
+/* initialize our config and fetch it */
+conf = ap_set_config_vectors(cmd->server, new_dir_conf, arg,
+ &dbd_module, cmd->pool);
+
+errmsg = ap_walk_config(cmd->directive->first_child, cmd, new_dir_con
Re: [PATCH] mod_dbd with more than one pool
I am in favor of this for 2.3/2.4 as well -- it is functionality that I have wanted (not enough to do, though...) for a while now. -Brian On Fri, Jan 30, 2009 at 6:20 PM, Graham Leggett wrote: > Kevac Marko wrote: > >> Once again I want to propose patch for mod_dbd module. This patch make >> possible to use more than one database pool. > > One of the things I needed to do earlier today was determine whether mod_dbd > could support more than one database pool, this patch answers that question. > > A definite +1 for httpd v2.3/2.4. > >> API is only slightly changed (new argument 'pool name' added). >> >> I am looking forward to comments and instructions how to add this to >> mainline http-2.2.x. > > If you can find a way to achieve this without the ABI changing, then it can > be backported to v2.2. Otherwise, it would be another reason to move on to > v2.4. > > Regards, > Graham > -- >
Re: [PATCH] mod_dbd with more than one pool
On Feb 11, 2009, at 6:22 PM, Graham Leggett wrote: Would it make sense for mod_memcache to become a provider beneath mod_socache, or am I missing something? mod_memcache really just provides the config "glue" for apr_memcache so that every module that wants to use apr_memcache doesn't have to write all the config "glue" themselves. Yeah, it could probably add some socache hooks as well. I'm adding some Lua glue right now. mod_memcache is already Apache licensed.
Re: [PATCH] mod_dbd with more than one pool
Brian Akins wrote: I was looking to do the same thing to mod_memcache (which should be imported into trunk, IMO...) Would it make sense for mod_memcache to become a provider beneath mod_socache, or am I missing something? Regards, Graham -- smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: Thus old functions and old configuration is preserved. What so you think? That sounds to me like it would be safe to backport such a thing to v2.2. +1. Regards, Graham -- smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] mod_dbd with more than one pool
On Thu, 12 Feb 2009 00:29:59 +0300 Kevac Marko wrote: > In httpd.conf you can create named pool inside or > without. In second case pool_name is DBD_DEFAULT_POOL_NAME. > > Thus old functions and old configuration is preserved. > > What so you think? > > Patch is ready, but it needs some testing before posting. Sounds OK in principle for trunk. If you want to post a patch against trunk, I'll try and find the time to review it. -- Nick Kew Application Development with Apache - the Apache Modules Book http://www.apachetutor.org/
Re: [PATCH] mod_dbd with more than one pool
On Thu, Feb 12, 2009 at 12:43 AM, Brian Akins wrote: > > I was looking to do the same thing to mod_memcache (which should be imported > into trunk, IMO...) kni...@juffin:~/micex/git/apache$ tree modules/memcache/ modules/memcache/ |-- SConscript |-- mod_memcache.c `-- mod_memcache.h Same as mod_dbd :-) -- Marko Kevac
Re: [PATCH] mod_dbd with more than one pool
On 2/11/09 4:29 PM, "Kevac Marko" wrote: > What so you think? > > Patch is ready, but it needs some testing before posting. +1 I was looking to do the same thing to mod_memcache (which should be imported into trunk, IMO...)
Re: [PATCH] mod_dbd with more than one pool
Now for every exported function we have pair of functions that accepts
pool_name and old one, which is just wrapper:
DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open_pool(apr_pool_t *pool, server_rec *s,
const char *pool_name);
DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s);
APR_DECLARE_OPTIONAL_FN(ap_dbd_t*, ap_dbd_open_pool, (apr_pool_t*,
server_rec*, const char*));
APR_DECLARE_OPTIONAL_FN(ap_dbd_t*, ap_dbd_open, (apr_pool_t*, server_rec*));
DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s)
{
return ap_dbd_open_pool(pool, s, DBD_DEFAULT_POOL_NAME);
}
In httpd.conf you can create named pool inside or
without. In second case pool_name is DBD_DEFAULT_POOL_NAME.
Thus old functions and old configuration is preserved.
What so you think?
Patch is ready, but it needs some testing before posting.
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: Once again I want to propose patch for mod_dbd module. This patch make possible to use more than one database pool. One of the things I needed to do earlier today was determine whether mod_dbd could support more than one database pool, this patch answers that question. A definite +1 for httpd v2.3/2.4. API is only slightly changed (new argument 'pool name' added). I am looking forward to comments and instructions how to add this to mainline http-2.2.x. If you can find a way to achieve this without the ABI changing, then it can be backported to v2.2. Otherwise, it would be another reason to move on to v2.4. Regards, Graham -- smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] mod_dbd with more than one pool
Kevac Marko wrote: API is only slightly changed (new argument 'pool name' added). I am looking forward to comments and instructions how to add this to mainline http-2.2.x. You can't change the ABI, let alone the ABI, within 2.2.x. That would break the contract with third-party developers. To introduce a new API, you'd have to use a wrapper. You can propose changes in trunk, which will then feed into 2.4. Your biggest hurdle there is to get developer attention and review. Take a look at how an API changes. Contrast http://svn.apache.org/viewvc?view=rev&revision=730296 http://svn.apache.org/viewvc?view=rev&revision=732583 and see how trunk changes ABI (but preserves API) while 2.2.x preserves the ABI too. -- Nick Kew
