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

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

From: Eric Gillespie <epg_at_pretzelnet.org>
Date: 2007-04-10 20:46:59 CEST

Malcolm Rowe <malcolm-svn-dev@farside.org.uk> writes:

> On Fri, Apr 06, 2007 at 04:20:24PM -0700, Eric Gillespie wrote:

> > Stop using apr_dbm to manage the DAV activities database which maps
>
> > Make the location of this directory configurable with the new
>
> I think these should probably be two separate patches.

Hrm, well, OK. Here's the updated patch to handle just the first
part. Tests running over this now, seem to be doing fine.

[[[
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.

* subversion/mod_dav_svn/activity.c
  (ACTIVITY_DB): Change to 'dav/activities.d' so as not to conflict
  with the DBM file 'dav/activities'.
  (_read_txn): New helper 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/activity.c
===================================================================
--- subversion/mod_dav_svn/activity.c (revision 24508)
+++ subversion/mod_dav_svn/activity.c (working copy)
Index: activity.c
===================================================================
--- activity.c (revision 24508)
+++ 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,52 @@
 #include "dav_svn.h"
 
 
-#define ACTIVITY_DB "dav/activities"
+#define ACTIVITY_DB "dav/activities.d"
 
 
-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_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_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ return _read_txn(repos, pathname);
 }
 
 
@@ -76,11 +84,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 +94,11 @@
      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_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ 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 +124,7 @@
               err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                          "could not open transaction.",
                                          repos->pool);
- goto cleanup;
+ return err;
             }
         }
       else
@@ -142,24 +135,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 +157,56 @@
                         const char *activity_id,
                         const char *txn_name)
 {
- apr_dbm_t *dbm;
+ const char *pathname;
+ svn_error_t *err;
   apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
+ 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(svn_path_join_many(repos->pool,
+ repos->fs_path,
+ ACTIVITY_DB,
+ NULL),
+ 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_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ 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 Tue Apr 10 20:48:11 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.