Did you run any tests to observe the alleged contention? The dbm database is very fast. I'd be surprised that contention occurs in any typical workload.
Cheers, -g On Mon, Oct 17, 2022 at 4:48 AM <yla...@apache.org> wrote: > Author: ylavic > Date: Mon Oct 17 09:48:11 2022 > New Revision: 1904638 > > URL: http://svn.apache.org/viewvc?rev=1904638&view=rev > Log: > mod_dav: Allow to disable lock discovery via an DAVLockDiscovery > expression. > > mod_dav-fs scales badly when a few clients run PROPFIND requests to > discover > directory content. Each PROPFIND involves lockdiscovery, which in turn > waits > for a locked access to the file containing the lock database. Performances > quickly drop because of lock contention on this file. > > Add a DAVLockDiscovery configuration directive that allows lockdiscovery > to be > disabled. Its argument is an Apache expression so that flexible > configuration > are possible (per-request). > > When lock discovery is disabled, an empty lockdiscovery property is > returned on > POPRFIND methods, just like if no lock was set on the object. That should > cause > no regression, since a client cannot rely on lockdiscovery to decide when a > file should be accessed, the LOCK methood must be used. > > If DAVLockDiscovery is not specified, the behavior is unchanged. > > > PR 66313. > Submitted by: Emmanuel Dreyfus <manu netbsd.org> > Reviewed by: ylavic > > > Added: > httpd/httpd/trunk/changes-entries/DAVLockDiscovery.txt > Modified: > httpd/httpd/trunk/modules/dav/main/mod_dav.c > httpd/httpd/trunk/modules/dav/main/mod_dav.h > httpd/httpd/trunk/modules/dav/main/props.c > > Added: httpd/httpd/trunk/changes-entries/DAVLockDiscovery.txt > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/DAVLockDiscovery.txt?rev=1904638&view=auto > > ============================================================================== > --- httpd/httpd/trunk/changes-entries/DAVLockDiscovery.txt (added) > +++ httpd/httpd/trunk/changes-entries/DAVLockDiscovery.txt Mon Oct 17 > 09:48:11 2022 > @@ -0,0 +1,2 @@ > + *) mod_dav: Allow to disable lock discovery via an DAVLockDiscovery > + expression (per-request). PR 66313. [Emmanuel Dreyfus <manu > netbsd.org>] > > Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1904638&r1=1904637&r2=1904638&view=diff > > ============================================================================== > --- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original) > +++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Mon Oct 17 09:48:11 2022 > @@ -83,6 +83,7 @@ typedef struct { > const char *dir; > int locktimeout; > int allow_depthinfinity; > + ap_expr_info_t *allow_lockdiscovery; > > } dav_dir_conf; > > @@ -203,6 +204,8 @@ static void *dav_merge_dir_config(apr_po > newconf->dir = DAV_INHERIT_VALUE(parent, child, dir); > newconf->allow_depthinfinity = DAV_INHERIT_VALUE(parent, child, > allow_depthinfinity); > + newconf->allow_lockdiscovery = DAV_INHERIT_VALUE(parent, child, > + allow_lockdiscovery); > > return newconf; > } > @@ -301,6 +304,30 @@ static const char *dav_cmd_davdepthinfin > } > > /* > + * Command handler for the DAVLockDiscovery directive, which is TAKE1. > + */ > +static const char *dav_cmd_davlockdiscovery(cmd_parms *cmd, void *config, > + const char *arg) > +{ > + dav_dir_conf *conf = (dav_dir_conf *)config; > + > + if (strncasecmp(arg, "expr=", 5) == 0) { > + const char *err; > + if ((arg[5] == '\0')) > + return "missing condition"; > + conf->allow_lockdiscovery = ap_expr_parse_cmd(cmd, &arg[5], > + > AP_EXPR_FLAG_DONT_VARY, > + &err, NULL); > + if (err) > + return err; > + } else { > + return "error in condition clause"; > + } > + > + return NULL; > +} > + > +/* > * Command handler for DAVMinTimeout directive, which is TAKE1 > */ > static const char *dav_cmd_davmintimeout(cmd_parms *cmd, void *config, > @@ -1450,7 +1477,7 @@ static dav_error *dav_gen_supported_live > } > > /* open the property database (readonly) for the resource */ > - if ((err = dav_open_propdb(r, lockdb, resource, 1, NULL, > + if ((err = dav_open_propdb(r, lockdb, resource, DAV_PROPDB_RO, NULL, > &propdb)) != NULL) { > if (lockdb != NULL) > (*lockdb->hooks->close_lockdb)(lockdb); > @@ -2045,6 +2072,8 @@ static void dav_cache_badprops(dav_walke > static dav_error * dav_propfind_walker(dav_walk_resource *wres, int > calltype) > { > dav_walker_ctx *ctx = wres->walk_ctx; > + dav_dir_conf *conf; > + int flags = DAV_PROPDB_RO; > dav_error *err; > dav_propdb *propdb; > dav_get_props_result propstats = { 0 }; > @@ -2068,6 +2097,20 @@ static dav_error * dav_propfind_walker(d > return NULL; > } > > + conf = ap_get_module_config(ctx->r->per_dir_config, &dav_module); > + if (conf && conf->allow_lockdiscovery) { > + const char *errstr = NULL; > + int eval = ap_expr_exec(ctx->r, conf->allow_lockdiscovery, > &errstr); > + if (errstr) { > + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, > APLOGNO(00623) > + "Failed to evaluate expression (%s) - > ignoring", > + errstr); > + } else { > + if (!eval) > + flags |= DAV_PROPDB_DISABLE_LOCKDISCOVERY; > + } > + } > + > /* > ** Note: ctx->doc can only be NULL for DAV_PROPFIND_IS_ALLPROP. Since > ** dav_get_allprops() does not need to do namespace translation, > @@ -2077,7 +2120,7 @@ static dav_error * dav_propfind_walker(d > ** the resource, however, since we are opening readonly. > */ > err = dav_popen_propdb(ctx->scratchpool, > - ctx->r, ctx->w.lockdb, wres->resource, 1, > + ctx->r, ctx->w.lockdb, wres->resource, flags, > ctx->doc ? ctx->doc->namespaces : NULL, > &propdb); > if (err != NULL) { > /* ### do something with err! */ > @@ -2463,7 +2506,8 @@ static int dav_method_proppatch(request_ > return dav_handle_err(r, err, NULL); > } > > - if ((err = dav_open_propdb(r, NULL, resource, 0, doc->namespaces, > + if ((err = dav_open_propdb(r, NULL, resource, > + DAV_PROPDB_NONE, doc->namespaces, > &propdb)) != NULL) { > /* undo any auto-checkout */ > dav_auto_checkin(r, resource, 1 /*undo*/, 0 /*unlock*/, &av_info); > @@ -5220,6 +5264,11 @@ static const command_rec dav_cmds[] = > ACCESS_CONF|RSRC_CONF, > "allow Depth infinity PROPFIND requests"), > > + /* per directory/location, or per server */ > + AP_INIT_TAKE1("DAVLockDiscovery", dav_cmd_davlockdiscovery, NULL, > + ACCESS_CONF|RSRC_CONF, > + "allow lock discovery by PROPFIND requests"), > + > { NULL } > }; > > > Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.h?rev=1904638&r1=1904637&r2=1904638&view=diff > > ============================================================================== > --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original) > +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Mon Oct 17 09:48:11 2022 > @@ -1691,12 +1691,15 @@ struct dav_hooks_locks > > typedef struct dav_propdb dav_propdb; > > +#define DAV_PROPDB_NONE 0 > +#define DAV_PROPDB_RO 1 > +#define DAV_PROPDB_DISABLE_LOCKDISCOVERY 2 > > DAV_DECLARE(dav_error *) dav_open_propdb( > request_rec *r, > dav_lockdb *lockdb, > const dav_resource *resource, > - int ro, > + int flags, > apr_array_header_t *ns_xlate, > dav_propdb **propdb); > > @@ -1705,7 +1708,7 @@ DAV_DECLARE(dav_error *) dav_popen_propd > request_rec *r, > dav_lockdb *lockdb, > const dav_resource *resource, > - int ro, > + int flags, > apr_array_header_t *ns_xlate, > dav_propdb **propdb); > > > Modified: httpd/httpd/trunk/modules/dav/main/props.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/props.c?rev=1904638&r1=1904637&r2=1904638&view=diff > > ============================================================================== > --- httpd/httpd/trunk/modules/dav/main/props.c (original) > +++ httpd/httpd/trunk/modules/dav/main/props.c Mon Oct 17 09:48:11 2022 > @@ -185,6 +185,8 @@ struct dav_propdb { > > dav_buffer wb_lock; /* work buffer for lockdiscovery > property */ > > + int flags; /* ro, disable lock discovery */ > + > /* if we ever run a GET subreq, it will be stored here */ > request_rec *subreq; > > @@ -351,6 +353,11 @@ static dav_error * dav_insert_coreprop(d > switch (propid) { > > case DAV_PROPID_CORE_lockdiscovery: > + if (propdb->flags & DAV_PROPDB_DISABLE_LOCKDISCOVERY) { > + value = ""; > + break; > + } > + > if (propdb->lockdb != NULL) { > dav_lock *locks; > > @@ -522,17 +529,18 @@ static dav_error *dav_really_open_db(dav > > DAV_DECLARE(dav_error *)dav_open_propdb(request_rec *r, dav_lockdb > *lockdb, > const dav_resource *resource, > - int ro, > + int flags, > apr_array_header_t * ns_xlate, > dav_propdb **p_propdb) > { > - return dav_popen_propdb(r->pool, r, lockdb, resource, ro, ns_xlate, > p_propdb); > + return dav_popen_propdb(r->pool, r, lockdb, resource, > + flags, ns_xlate, p_propdb); > } > > DAV_DECLARE(dav_error *)dav_popen_propdb(apr_pool_t *p, > request_rec *r, dav_lockdb > *lockdb, > const dav_resource *resource, > - int ro, > + int flags, > apr_array_header_t * ns_xlate, > dav_propdb **p_propdb) > { > @@ -559,6 +567,8 @@ DAV_DECLARE(dav_error *)dav_popen_propdb > > propdb->lockdb = lockdb; > > + propdb->flags = flags; > + > /* always defer actual open, to avoid expense of accessing db > * when only live properties are involved > */ > > >
