I've updated the patch to account for Malcolm's and Ben's
comments. The activity file is created as a temporary file and
then renamed into place with the MD5-encoded file name, and
read_txn (renamed :) handles ESTALE. The transaction name is now
terminated with a newline.
[[[
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'.
(escape_activity): New helper to escape activity for safe use as a
file name.
(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 24594)
+++ subversion/mod_dav_svn/activity.c (working copy)
@@ -16,11 +16,16 @@
* ====================================================================
*/
-#include <apr_dbm.h>
+#include <errno.h>
+#include <apr_md5.h>
+
#include <httpd.h>
#include <mod_dav.h>
+#include "svn_error.h"
+#include "svn_io.h"
+#include "svn_md5.h"
#include "svn_path.h"
#include "svn_fs.h"
#include "svn_repos.h"
@@ -28,59 +33,111 @@
#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 *
+escape_activity(const char *activity_id, apr_pool_t *pool)
{
- apr_dbm_t *dbm;
- apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
- const char *txn_name = NULL;
+ unsigned char digest[APR_MD5_DIGESTSIZE];
+ apr_md5(digest, activity_id, strlen(activity_id));
+ return svn_md5_digest_to_cstring_display(digest, pool);
+}
- 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)
+
+/* FSFS transaction ids:
+ 19 bytes for svn_revnum_t (room for 32 or 64 bit values)
+ + 1 byte for '-'
+ + 5 bytes (up to 99999)
+ + 1 terminating null / newline
+ = 26 bytes
+ BDB transaction ids are base-36, bounded by bdb MAX_KEY_SIZE, but
+ realistically, 26 bytes ought to be enough.
+ */
+#define TXN_LEN 26
+
+static const char *
+read_txn(const dav_svn_repos *repos, const char *pathname)
+{
+ apr_file_t *activity_file;
+ apr_pool_t *iterpool = svn_pool_create(repos->pool);
+ apr_size_t len = TXN_LEN;
+ svn_error_t *err = SVN_NO_ERROR;
+ char *txn_name = apr_palloc(repos->pool, len);;
+ int i;
+
+ /* Try up to 10 times to read the txn name, retrying on ESTALE
+ (stale NFS file handle because of dav_svn__store_activity
+ renaming the activity file into place).
+ */
+ for (i = 0; i < 10; i++)
{
- /* ### let's just assume that any error means the DB doesn't exist,
- ### therefore, the activity/transaction doesn't exist */
- return NULL;
- }
+ svn_error_clear(err);
+ svn_pool_clear(iterpool);
- 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)
+ err = svn_io_file_open(&activity_file, pathname, APR_READ,
+ APR_OS_DEFAULT, iterpool);
+ if (err && err->apr_err != APR_SUCCESS)
{
- /* ### again: assume failure means it doesn't exist */
- apr_dbm_close(dbm);
+#ifdef ESTALE
+ if (APR_TO_OS_ERROR(err->apr_err) == ESTALE)
+ continue;
+#endif
+ /* ### let's just assume that any error means the
+ ### activity/transaction doesn't exist */
return NULL;
}
- txn_name = apr_pstrdup(repos->pool, value.dptr); /* null-term'd */
- apr_dbm_freedatum(dbm, value);
+
+ err = svn_io_read_length_line(activity_file, txn_name, &len, NULL);
+ if (err && err->apr_err != APR_SUCCESS)
+ {
+#ifdef ESTALE
+ if (APR_TO_OS_ERROR(err->apr_err) == ESTALE)
+ continue;
+#endif
+ return NULL;
+ }
+
+ err = svn_io_file_close(activity_file, iterpool);
+ if (err && err->apr_err != APR_SUCCESS)
+ {
+#ifdef ESTALE
+ if (APR_TO_OS_ERROR(err->apr_err) != ESTALE)
+#endif
+ return NULL;
+ }
+
+ break;
}
+ svn_pool_destroy(iterpool);
- apr_dbm_close(dbm);
+ if (err)
+ {
+ svn_error_clear(err);
+ return NULL;
+ }
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,
+ escape_activity(activity_id, repos->pool),
+ NULL);
+ return read_txn(repos, pathname);
+}
+
+
dav_error *
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 +146,12 @@
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,
+ escape_activity(activity_id, repos->pool),
+ 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 +177,7 @@
err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
"could not open transaction.",
repos->pool);
- goto cleanup;
+ return err;
}
}
else
@@ -142,24 +188,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,40 +210,72 @@
const char *activity_id,
const char *txn_name)
{
- apr_dbm_t *dbm;
+ const char *final_path, *tmp_path;
+ svn_error_t *err;
apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
+ apr_file_t *activity_file;
- 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);
- if (status != APR_SUCCESS)
+ /* 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);
+
+ final_path = svn_path_join_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ escape_activity(activity_id, repos->pool),
+ NULL);
+ err = svn_io_open_unique_file2(&activity_file, &tmp_path, final_path,
+ ".tmp", svn_io_file_del_none, repos->pool);
+ if (err)
{
- svn_error_t *serr = svn_error_wrap_apr(status, "Can't open activity db");
+ svn_error_t *serr = svn_error_quick_wrap(err, "Can't open activity db");
+ return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "could not open files.",
+ repos->pool);
+ }
+ if (apr_file_printf(activity_file, "%s\n", txn_name) < 1)
+ {
+ svn_error_t *serr =
+ svn_error_wrap_apr(status, "Can't write to activity db");
+
+ /* Try to remove the tmp file, but we already have an error... */
+ svn_error_clear(svn_io_remove_file(tmp_path, repos->pool));
+
return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not open dbm files.",
+ "could not write 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);
+ status = apr_file_close(activity_file);
if (status != APR_SUCCESS)
{
svn_error_t *serr =
svn_error_wrap_apr(status, "Can't close activity db");
+ svn_error_clear(svn_io_remove_file(tmp_path, repos->pool));
+
return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not close dbm files.",
+ "could not close files.",
repos->pool);
}
+ err = svn_io_file_rename(tmp_path, final_path, repos->pool);
+ if (err)
+ {
+
+ svn_error_clear(svn_io_remove_file(tmp_path, repos->pool));
+
+ return dav_svn__convert_err(err, HTTP_INTERNAL_SERVER_ERROR,
+ "could not close files.",
+ repos->pool);
+ }
+
return NULL;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 16 23:23:27 2007