Karl Fogel wrote:
> Blair Zajac <blair@orcaware.com> writes:
>> I'm making a svn filesystem network aware using Ice RPC and one of the
>> things I'm running into is that FSFS transactions are not unique. BDB
>> seems to generate transaction names via a sequence, so this isn't an
>> issue.
>>
>> I have the ability for a client to begin a transaction remotely and
>> then refer to that transaction to do work. In FSFS, if a client
>> begins a transaction, gives the transaction name to another process,
>> which then closes it, and a third process begins a new transaction
>> based on the same revision, then it will get the same transaction name
>> and the owner of the original transaction will be operating inside a
>> different transaction and will be able to abort the transaction, which
>> it should not be able to do.
>>
>> So I'd like to make the transaction names unique, using something like
>>
>> "%s-%05ds-%s-%05d" % (apr_gethostname(),
>> apr_uid_current(),
>> apr_time_now(),
>> i)
>>
>> where i is incremented until it finds a non-existent directory name.
>>
>> The current transaction name is based off the revision number, is this
>> necessary? Should we keep it? Does changing the transaction name
>> have any other impact?
>
> +1 on making transaction names unique -- in fact, maybe we
> should take this opportunity to add that as an API promise too. We
> might well add features to Subversion someday that involve the use of
> long-lived transactions.
>
> I don't see any reason why the transaction number needs to be based
> off the revision number, assuming it has another way of associating
> itself with its base revision (which it does, right?).
Below is a patch that generates transaction names in the form
<hostname>_<pid>_<time>_<uniquifier>. The test suite is running and has passed
35/50 tests.
After looking at this, I'm wondering if maybe we should use sqlite to store a
sequence value in it and use the same key incrementing approach that the BDB
backend uses to generate transaction names. The current value of this sequence
would be dumped along with the repository and imported, thereby preventing the
reuse of a transaction name. I believe that if you dump and load a BDB
transaction, then currently, sequence is reset.
Using sqlite would be more work, since I'd have to familiarize myself with it,
but it may be a cleaner way to go.
If we did go that way, would we drop the key generation from the BDB tables?
Anyway, here's the patch that can go in as a temporary measure, until we decide
on a better approach.
[[[
Generate FSFS transaction names that should ideally be unique and not
reused by replacing the revision number that the transaction is based
off with the hostname, process ID and current time, in the form
<hostname>_<pid>_<time>_<uniquifier>.
* subversion/libsvn_fs_fs/fs.h
Include apr_network_io.h to pick up APRMAXHOSTLEN.
(fs_fs_shared_txn_data_t.txn_id):
Increase the length of this field to APRMAXHOSTLEN+64.
* subversion/libsvn_fs_fs/fs_fs.c
(create_txn_dir):
Create the new transaction names using the hostname, the process
ID and the current epoch time in microseconds since.
]]]
Index: subversion/libsvn_fs_fs/fs.h
===================================================================
--- subversion/libsvn_fs_fs/fs.h (revision 25401)
+++ subversion/libsvn_fs_fs/fs.h (working copy)
@@ -22,6 +22,7 @@
#include <apr_hash.h>
#include <apr_md5.h>
#include <apr_thread_mutex.h>
+#include <apr_network_io.h>
#include "svn_fs.h"
@@ -62,10 +63,13 @@
transaction. */
struct fs_fs_shared_txn_data_t *next;
- /* This transaction's ID. This is in the form <rev>-<uniqueifier>,
- where <uniqueifier> runs from 0-99999 (see create_txn_dir() in
- fs_fs.c). */
- char txn_id[18];
+ /* This transaction's ID. This is in the form
+ <hostname>_<pid>_<time>_<uniquifier>, where <uniqueifier> runs
+ from 0-99999 (see create_txn_dir() in fs_fs.c). Reading the ID
+ from left to right with 32 bit PIDs and microsecond times, the
+ minimum length is APRMAXHOSTLEN + 1 + 10 + 1 + 16 + 1 + 5. Here,
+ add some extra space for future growth. */
+ char txn_id[APRMAXHOSTLEN+64];
/* Whether the transaction's prototype revision file is locked for
writing by any thread in this process (including the current
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 25401)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -29,6 +29,7 @@
#include <apr_lib.h>
#include <apr_md5.h>
#include <apr_thread_mutex.h>
+#include <apr_network_io.h>
#include "svn_pools.h"
#include "svn_fs.h"
@@ -3161,21 +3162,42 @@
create_txn_dir(const char **id_p, svn_fs_t *fs, svn_revnum_t rev,
apr_pool_t *pool)
{
+ char hostname_str[APRMAXHOSTLEN + 1] = { 0 };
+ pid_t process_id;
+ apr_time_t now;
unsigned int i;
apr_pool_t *subpool;
const char *unique_path, *prefix;
+ char *p;
+ apr_status_t apr_err;
- /* Try to create directories named "<txndir>/<rev>-<uniquifier>.txn". */
- prefix = svn_path_join_many(pool, fs->path, PATH_TXNS_DIR,
- apr_psprintf(pool, "%ld", rev), NULL);
+ /* Try to create transaction directories named
+ "<txndir>/<hostname>_<pid>_<time>_<uniquifier>.txn". */
+ apr_err = apr_gethostname(hostname_str, sizeof(hostname_str), pool);
+ if (apr_err)
+ return svn_error_wrap_apr(apr_err, _("Can't get local hostname"));
+
+ /* Transaction names cannot have periods in them, so convert them
+ all into hyphens. */
+ for (p=hostname_str; *p; ++p)
+ if ('.' == *p)
+ *p = '-';
+ process_id = getpid();
+ now = apr_time_now();
+
+ prefix = svn_path_join_many(pool, fs->path, PATH_TXNS_DIR,
+ apr_psprintf(pool, "%s_%05d_%ld",
+ hostname_str, process_id, now),
+ NULL);
subpool = svn_pool_create(pool);
for (i = 1; i <= 99999; i++)
{
svn_error_t *err;
svn_pool_clear(subpool);
- unique_path = apr_psprintf(subpool, "%s-%u" PATH_EXT_TXN, prefix, i);
+ unique_path = apr_psprintf(subpool, "%s_%u" PATH_EXT_TXN, prefix, i);
+
err = svn_io_dir_make(unique_path, APR_OS_DEFAULT, subpool);
if (! err)
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 14 03:06:52 2007