[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Re: Is mod_dav_svn safe for use in a threaded MPM?

From: Eric Gillespie <epg_at_pretzelnet.org>
Date: 2007-04-07 01:20:24 CEST

OK, i've implemented my proposal, except i have not removed the
'dav' directory. It's part of the public svn_repos_t structure.
Can i just leave that field there (and continue to fill it in),
but just stop creating the directory?

[[[
Stop using apr_dbm to manage the DAV activities database which maps
DAV activity ids to Subversion transaction names. Instead, use a
simple directory structure, with activity ids as file names and
transaction names as file contents.

Make the location of this directory configurable with the new
SVNActivitiesDB configuration directive. If set, it will be used as
the activity db directly for SVNPath repositories. For SVNParentPath
repositories, it will contain activity db directories named after the
repository basename. If this directive is not specified, use a
directory called "dav-activities" at the top of the repository.

* subversion/mod_dav_svn/mod_dav_svn.c
  (dir_conf_t): Add activities_db member.
  (merge_dir_config): Merge in activities_db field.
  (SVNActivitiesDB_cmd): Implement SVNActivitiesDB.
  (dav_svn__get_activities_db): Expose new field.
  (cmds): Declare SVNActivitiesDB directive.

* subversion/mod_dav_svn/dav_svn.h
  (dav_svn_repos): Add activities_db member.
  (dav_svn__get_activities_db): Declare.

* subversion/mod_dav_svn/repos.c
  (DEFAULT_ACTIVITY_DB): New macro.
  (get_resource): Set repos->activities_db based on activities_db
  config and maybe DEFAULT_ACTIVITY_DB.

* subversion/mod_dav_svn/activity.c
  (_read_txn): New helper function to return the txn from an activity
  file.
  (dav_svn__get_txn): Make this a simple wrapper around _read_txn .
  (dav_svn__delete_activity): Use _read_txn to get the transaction
  name; delete the file from the activities directory.
  (dav_svn__store_activity): Create the activities directory if it
  does not exist and create the activity file.
]]]

Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c (revision 24488)
+++ subversion/mod_dav_svn/mod_dav_svn.c (working copy)
@@ -67,6 +67,7 @@
   enum conf_flag list_parentpath; /* whether to allow GET of parentpath */
   const char *root_dir; /* our top-level directory */
   const char *master_uri; /* URI to the master SVN repos */
+ const char *activities_db; /* path to activities database(s) */
 } dir_conf_t;
 
 
@@ -156,6 +157,7 @@
 
   newconf->fs_path = INHERIT_VALUE(parent, child, fs_path);
   newconf->master_uri = INHERIT_VALUE(parent, child, master_uri);
+ newconf->activities_db = INHERIT_VALUE(parent, child, activities_db);
   newconf->repo_name = INHERIT_VALUE(parent, child, repo_name);
   newconf->xslt_uri = INHERIT_VALUE(parent, child, xslt_uri);
   newconf->fs_parent_path = INHERIT_VALUE(parent, child, fs_parent_path);
@@ -192,6 +194,17 @@
 
 
 static const char *
+SVNActivitiesDB_cmd(cmd_parms *cmd, void *config, const char *arg1)
+{
+ dir_conf_t *conf = config;
+
+ conf->activities_db = apr_pstrdup(cmd->pool, arg1);
+
+ return NULL;
+}
+
+
+static const char *
 SVNIndexXSLT_cmd(cmd_parms *cmd, void *config, const char *arg1)
 {
   dir_conf_t *conf = config;
@@ -454,6 +467,16 @@
 }
 
 
+const char *
+dav_svn__get_activities_db(request_rec *r)
+{
+ dir_conf_t *conf;
+
+ conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
+ return conf->activities_db;
+}
+
+
 static void
 merge_xml_filter_insert(request_rec *r)
 {
@@ -619,6 +642,12 @@
   AP_INIT_TAKE1("SVNMasterURI", SVNMasterURI_cmd, NULL, ACCESS_CONF,
                 "specifies a URI to access a master Subversion repository"),
 
+ /* per directory/location */
+ AP_INIT_TAKE1("SVNActivitiesDB", SVNActivitiesDB_cmd, NULL, ACCESS_CONF,
+ "specifies the location in the filesystem in which the "
+ "activities database(s) should be stored"),
+
+
   { NULL }
 };
 
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 24488)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -109,6 +109,9 @@
   /* is the client a Subversion client? */
   svn_boolean_t is_svn_client;
 
+ /* The path to the activities db */
+ const char *activities_db;
+
 } dav_svn_repos;
 
 
@@ -296,6 +299,9 @@
 /* Return the master URI (for mirroring) */
 const char * dav_svn__get_master_uri(request_rec *r);
 
+/* Return the activities db */
+const char * dav_svn__get_activities_db(request_rec *r);
+
 /* Return the root directory */
 const char * dav_svn__get_root_dir(request_rec *r);
 
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 24488)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -44,6 +44,9 @@
 #include "dav_svn.h"
 
 
+#define DEFAULT_ACTIVITY_DB "dav-activities"
+
+
 struct dav_stream {
   const dav_resource *res;
 
@@ -1572,6 +1575,18 @@
   /* Is autoversioning active in this repos? */
   repos->autoversioning = dav_svn__get_autoversioning_flag(r);
 
+ /* Path to activities database */
+ repos->activities_db = dav_svn__get_activities_db(r);
+ if (repos->activities_db == NULL)
+ repos->activities_db = svn_path_join(repos->fs_path,
+ DEFAULT_ACTIVITY_DB,
+ r->pool);
+ else if (fs_parent_path != NULL)
+ repos->activities_db = svn_path_join(repos->activities_db,
+ svn_path_basename(repos->fs_path,
+ r->pool),
+ r->pool);
+
   /* Remember various bits for later URL construction */
   repos->base_url = ap_construct_url(r->pool, "", r);
   repos->special_uri = dav_svn__get_special_uri(r);
Index: subversion/mod_dav_svn/activity.c
===================================================================
--- subversion/mod_dav_svn/activity.c (revision 24488)
+++ subversion/mod_dav_svn/activity.c (working copy)
@@ -21,6 +21,9 @@
 #include <httpd.h>
 #include <mod_dav.h>
 
+#include <apr_file_info.h>
+#include <apr_file_io.h>
+
 #include "svn_path.h"
 #include "svn_fs.h"
 #include "svn_repos.h"
@@ -28,47 +31,48 @@
 #include "dav_svn.h"
 
 
-#define ACTIVITY_DB "dav/activities"
-
-
-const char *
-dav_svn__get_txn(const dav_svn_repos *repos, const char *activity_id)
+static const char *
+_read_txn(const dav_svn_repos *repos, const char *pathname)
 {
- apr_dbm_t *dbm;
   apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
- const char *txn_name = NULL;
+ apr_finfo_t finfo;
+ char *txn_name = NULL;
+ apr_file_t *activity_file;
+ apr_size_t bytes_read;
 
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_READONLY,
- APR_OS_DEFAULT, repos->pool);
- if (status != APR_SUCCESS)
+ /* ### let's just assume that any error means the
+ ### activity/transaction doesn't exist */
+# define _CHECK_STATUS if (status != APR_SUCCESS) return NULL
+
+ status = apr_stat(&finfo, pathname, APR_FINFO_SIZE, repos->pool);
+ _CHECK_STATUS;
+
+ status = apr_file_open(&activity_file, pathname, APR_READ | APR_BUFFERED,
+ APR_OS_DEFAULT, repos->pool);
+ _CHECK_STATUS;
+
+ txn_name = apr_palloc(repos->pool, finfo.size + 1); /* null-term'd */
+ status = apr_file_read_full(activity_file, txn_name, finfo.size, &bytes_read);
+ _CHECK_STATUS;
+ if (bytes_read != finfo.size)
     {
- /* ### let's just assume that any error means the DB doesn't exist,
- ### therefore, the activity/transaction doesn't exist */
       return NULL;
     }
+ status = apr_file_close(activity_file);
+ _CHECK_STATUS;
+# undef _CHECK_STATUS
 
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- if (apr_dbm_exists(dbm, key))
- {
- status = apr_dbm_fetch(dbm, key, &value);
- if (status != APR_SUCCESS)
- {
- /* ### again: assume failure means it doesn't exist */
- apr_dbm_close(dbm);
- return NULL;
- }
- txn_name = apr_pstrdup(repos->pool, value.dptr); /* null-term'd */
- apr_dbm_freedatum(dbm, value);
- }
+ txn_name[finfo.size] = '\0';
+ return txn_name;
+}
 
- apr_dbm_close(dbm);
 
- return txn_name;
+const char *
+dav_svn__get_txn(const dav_svn_repos *repos, const char *activity_id)
+{
+ const char *pathname;
+ pathname = svn_path_join(repos->activities_db, activity_id, repos->pool);
+ return _read_txn(repos, pathname);
 }
 
 
@@ -76,11 +80,8 @@
 dav_svn__delete_activity(const dav_svn_repos *repos, const char *activity_id)
 {
   dav_error *err = NULL;
- apr_dbm_t *dbm;
   apr_status_t status;
   const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
   svn_fs_txn_t *txn;
   const char *txn_name;
   svn_error_t *serr;
@@ -89,23 +90,10 @@
      404. If the transaction is not present or is immutable, return a
      204. For all other failures, return a 500. */
 
- /* Open the activities database. */
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_READWRITE,
- APR_OS_DEFAULT, repos->pool);
- if (status != APR_SUCCESS)
- return dav_new_error(repos->pool, HTTP_NOT_FOUND, 0,
- "could not open activities database.");
-
- /* Get the activity from the activity database. */
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- status = apr_dbm_fetch(dbm, key, &value);
- if (status == APR_SUCCESS)
- txn_name = value.dptr;
- else
+ pathname = svn_path_join(repos->activities_db, activity_id, repos->pool);
+ txn_name = _read_txn(repos, pathname);
+ if (txn_name == NULL)
     {
- apr_dbm_close(dbm);
       return dav_new_error(repos->pool, HTTP_NOT_FOUND, 0,
                            "could not find activity.");
     }
@@ -131,7 +119,7 @@
               err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                          "could not open transaction.",
                                          repos->pool);
- goto cleanup;
+ return err;
             }
         }
       else
@@ -142,24 +130,19 @@
               err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                          "could not abort transaction.",
                                          repos->pool);
- goto cleanup;
+ return err;
             }
         }
     }
-
+
   /* Finally, we remove the activity from the activities database. */
- status = apr_dbm_delete(dbm, key);
+ status = apr_file_remove(pathname, repos->pool);
   if (status)
     {
       err = dav_new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                           "unable to remove activity.");
- goto cleanup;
     }
 
- cleanup:
- apr_dbm_freedatum(dbm, value);
- apr_dbm_close(dbm);
-
   return err;
 }
 
@@ -169,37 +152,51 @@
                         const char *activity_id,
                         const char *txn_name)
 {
- apr_dbm_t *dbm;
- apr_status_t status;
   const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
+ svn_error_t *err;
+ apr_status_t status;
+ apr_file_t *activity_file;
+ apr_size_t nbytes, bytes_written;
 
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_RWCREATE,
- APR_OS_DEFAULT, repos->pool);
+ /* Create activities directory if it does not yet exist. */
+ err = svn_io_make_dir_recursively(repos->activities_db, repos->pool);
+ if (err != NULL)
+ return dav_svn__convert_err(err, HTTP_INTERNAL_SERVER_ERROR,
+ "could not open files.",
+ repos->pool);
+
+ pathname = svn_path_join(repos->activities_db, activity_id, repos->pool);
+ status = apr_file_open(&activity_file, pathname, APR_WRITE | APR_CREATE,
+ APR_OS_DEFAULT, repos->pool);
   if (status != APR_SUCCESS)
     {
       svn_error_t *serr = svn_error_wrap_apr(status, "Can't open activity db");
 
       return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not open dbm files.",
+ "could not open files.",
                                   repos->pool);
     }
 
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- value.dptr = (char *)txn_name;
- value.dsize = strlen(txn_name) + 1; /* null-term'd */
- status = apr_dbm_store(dbm, key, value);
- apr_dbm_close(dbm);
+ nbytes = strlen(txn_name);
+ status = apr_file_write_full(activity_file, txn_name, nbytes,
+ &bytes_written);
+ if (status != APR_SUCCESS || bytes_written != nbytes)
+ {
+ svn_error_t *serr =
+ svn_error_wrap_apr(status, "Can't write to activity db");
+ return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "could not write files.",
+ repos->pool);
+ }
+
+ status = apr_file_close(activity_file);
   if (status != APR_SUCCESS)
     {
       svn_error_t *serr =
         svn_error_wrap_apr(status, "Can't close activity db");
 
       return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not close dbm files.",
+ "could not close files.",
                                   repos->pool);
     }
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 7 01:21:37 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.