You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2008/10/20 10:51:20 UTC
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>...
> +++ trunk/subversion/include/svn_error_codes.h Sat Oct 18 06:11:59 2008 (r33730)
> @@ -673,6 +673,11 @@ SVN_ERROR_START
> SVN_ERR_FS_CATEGORY_START + 47,
> "Filesystem upgrade is not supported")
>
> + /** @since New in 1.6. */
> + SVN_ERRDEF(SVN_ERR_FS_NO_SUCH_CHECKSUM_REP,
> + SVN_ERR_FS_CATEGORY_START + 49,
> + "Filesystem has no such checksum-representation index record")
This should be + 48.
>...
> +++ trunk/subversion/include/svn_fs.h Sat Oct 18 06:11:59 2008 (r33730)
> @@ -31,6 +31,7 @@
> #include "svn_delta.h"
> #include "svn_io.h"
> #include "svn_mergeinfo.h"
> +#include "svn_checksum.h"
Oof. This shoulda been in there already since svn_checksum_t was in use.
>...
> +++ trunk/subversion/libsvn_fs_base/bdb/dbt.c Sat Oct 18 06:11:59 2008 (r33730)
> @@ -18,6 +18,8 @@
> #include <stdlib.h>
> #include <string.h>
> #include <apr_pools.h>
> +#include <apr_md5.h>
> +#include <apr_sha1.h>
>
> #define APU_WANT_DB
> #include <apu_want.h>
> @@ -153,3 +155,20 @@ svn_fs_base__str_to_dbt(DBT *dbt, const
> svn_fs_base__set_dbt(dbt, str, strlen(str));
> return dbt;
> }
> +
> +DBT *
> +svn_fs_base__checksum_to_dbt(DBT *dbt, svn_checksum_t *checksum)
> +{
> + switch (checksum->kind)
> + {
> + case svn_checksum_md5:
> + svn_fs_base__set_dbt(dbt, checksum->digest, APR_MD5_DIGESTSIZE);
> + break;
> +
> + case svn_checksum_sha1:
> + svn_fs_base__set_dbt(dbt, checksum->digest, APR_SHA1_DIGESTSIZE);
> + break;
> + }
> +
> + return dbt;
> +}
Maybe export the DIGESTSIZE() macro out of libsvn_subr/checksum.c ??
>...
> +++ trunk/subversion/libsvn_fs_base/dag.c Sat Oct 18 06:11:59 2008 (r33730)
> @@ -33,6 +33,7 @@
> #include "reps-strings.h"
> #include "revs-txns.h"
> #include "id.h"
> +#include "fsguid.h"
>
> #include "util/fs_skels.h"
>
> @@ -42,6 +43,7 @@
> #include "bdb/copies-table.h"
> #include "bdb/reps-table.h"
> #include "bdb/strings-table.h"
> +#include "bdb/checksum-reps-table.h"
>
> #include "private/svn_fs_util.h"
> #include "../libsvn_fs/fs-loader.h"
> @@ -576,9 +578,15 @@ svn_fs_base__dag_set_proplist(dag_node_t
> trail_t *trail,
> apr_pool_t *pool)
> {
> + svn_error_t *err;
> node_revision_t *noderev;
> - const char *rep_key, *mutable_rep_key;
> + const char *rep_key, *mutable_rep_key, *dup_rep_key;
> svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> + svn_stream_t *wstream;
> + apr_size_t len;
> + skel_t *proplist_skel;
> + svn_stringbuf_t *raw_proplist_buf;
> + svn_checksum_t *checksum;
>
> /* Sanity check: this node better be mutable! */
> if (! svn_fs_base__dag_check_mutable(node, txn_id))
> @@ -595,6 +603,34 @@ svn_fs_base__dag_set_proplist(dag_node_t
> trail, pool));
> rep_key = noderev->prop_key;
>
> + /* Flatten the proplist into a string, and calculate its md5 hash. */
> + SVN_ERR(svn_fs_base__unparse_proplist_skel(&proplist_skel,
> + proplist, pool));
> + raw_proplist_buf = svn_fs_base__unparse_skel(proplist_skel, pool);
> + SVN_ERR(svn_checksum(&checksum, svn_checksum_sha1, raw_proplist_buf->data,
> + raw_proplist_buf->len, pool));
The comment is wrong.
> +
> + /* If the resulting property list is exactly the same as another
> + string in the database, just use the previously existing string
> + and get outta here. */
> + err = svn_fs_bdb__get_checksum_rep(&dup_rep_key, fs, checksum,
> + trail, pool);
> + if (! err)
> + {
> + if (noderev->prop_key)
> + SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, noderev->prop_key,
> + txn_id, trail, pool));
> + noderev->prop_key = dup_rep_key;
> + return svn_fs_bdb__put_node_revision(fs, node->id, noderev,
> + trail, pool);
> + }
> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
> + {
> + svn_error_clear(err);
> + err = SVN_NO_ERROR;
> + }
> + SVN_ERR(err);
This last part seems a bit easier to do:
else if (err)
{
if (err->apr_err != ...)
return err;
svn_error_clear(err);
}
I mean, the SVN_ERR(err) is only useful for the else-block. The first
if() means you don't need to check err again. etc... it is simply more
checking than needed :-P
>...
> + if (! err)
> + {
> + useless_data_key = noderev->edit_key;
> + err = svn_fs_base__reserve_fsguid(trail->fs, &data_key_uniquifier,
> + trail, pool);
> + }
> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
> + {
> + svn_error_clear(err);
> + err = SVN_NO_ERROR;
> + new_data_key = noderev->edit_key;
> + }
> + SVN_ERR(err);
Same here.
>...
> +++ trunk/subversion/libsvn_fs_base/fs.h Sat Oct 18 06:11:59 2008 (r33730)
> @@ -41,6 +41,9 @@ extern "C" {
> back-end's format. */
> #define SVN_FS_BASE__FORMAT_NUMBER 4
>
> +/* Minimum format number that supports representation sharing */
> +#define SVN_FS_BASE__MIN_REP_SHARING_FORMAT 4
> +
> /* Minimum format number that supports the 'miscellaneous' table */
> #define SVN_FS_BASE__MIN_MISCELLANY_FORMAT 4
Seems like these two MIN constants should be combined into one, for simplicity.
>...
> +++ trunk/subversion/libsvn_fs_base/notes/structure Sat Oct 18 06:11:59 2008 (r33730)
Maybe make FSGUID explicit?
Also: where to note about a format upgrade that will include FSGUID?
If you're going to migrate all key generation to FSGUID, then you'll
want to start FSGUID at max(all-next-key-columns). I think if you
start FSGUID at "0" like the patch is currently doing, you *might*
close off the door to broader use. Haven't thought it through yet tho.
But if true, then you're going to want to do some work to get that
max() before we cut 1.6.
>...
> +++ trunk/subversion/libsvn_fs_fs/rep-cache.c Sat Oct 18 06:11:59 2008 (r33730, copy of r33729, branches/fs-rep-sharing/subversion/libsvn_fs_fs/rep-cache.c)
>...
> +const char *upgrade_sql[] = { NULL,
> + "pragma auto_vacuum = 1;"
> + APR_EOL_STR
> + "create table rep_cache (hash text not null, "
> + " revision integer not null, "
> + " offset integer not null, "
> + " size integer not null, "
> + " expanded_size integer not null, "
> + " reuse_count integer not null); "
> + APR_EOL_STR
> + "create unique index i_hash on rep_cache(hash); "
Shouldn't need this custom index if you just mark the hash column as
the primary key (which would be nice for docco purposes anyways).
>...
> +svn_error_t *
> +svn_fs_fs__get_rep_reference(representation_t **rep,
> + svn_fs_t *fs,
> + svn_checksum_t *checksum,
> + apr_pool_t *pool)
> +{
> + fs_fs_data_t *ffd = fs->fsap_data;
> + svn_boolean_t have_row;
> + svn_sqlite__stmt_t *stmt;
> +
> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> + "select revision, offset, size, expanded_size from rep_cache "
> + "where hash = :1", pool));
I would recommend preparing this statement when you open the db. Then
each time this function is called, you can just bind some new values,
run the query, and finalize the statement. Definitely more efficient.
>...
> +svn_error_t *
> +svn_fs_fs__set_rep_reference(svn_fs_t *fs,
> + representation_t *rep,
> + svn_boolean_t reject_dup,
> + apr_pool_t *pool)
> +{
>...
> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> + "insert into rep_cache (hash, revision, offset, size, "
> + "expanded_size, reuse_count) "
> + "values (:1, :2, :3, :4, :5, 0);", pool));
Likewise.
>...
> +svn_error_t *
> +svn_fs_fs__inc_rep_reuse(svn_fs_t *fs,
> + representation_t *rep,
> + apr_pool_t *pool)
> +{
> + fs_fs_data_t *ffd = fs->fsap_data;
> + svn_boolean_t have_row;
> + svn_sqlite__stmt_t *stmt;
> +
> + /* Fetch the current count. */
> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> + "select reuse_count from rep_cache where hash = :1", pool));
>...
> + /* Update the reuse_count. */
> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> + "update rep_cache set reuse_count = :1 where hash = :2",
> + pool));
And two more :-)
>...
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include
subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr
subvers
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Greg Stein wrote:
> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>> ...
>
>> +++ trunk/subversion/include/svn_error_codes.h Sat Oct 18 06:11:59 2008 (r33730)
>> @@ -673,6 +673,11 @@ SVN_ERROR_START
>> SVN_ERR_FS_CATEGORY_START + 47,
>> "Filesystem upgrade is not supported")
>>
>> + /** @since New in 1.6. */
>> + SVN_ERRDEF(SVN_ERR_FS_NO_SUCH_CHECKSUM_REP,
>> + SVN_ERR_FS_CATEGORY_START + 49,
>> + "Filesystem has no such checksum-representation index record")
>
> This should be + 48.
r33801.
>> ...
>
>> +++ trunk/subversion/include/svn_fs.h Sat Oct 18 06:11:59 2008 (r33730)
>> @@ -31,6 +31,7 @@
>> #include "svn_delta.h"
>> #include "svn_io.h"
>> #include "svn_mergeinfo.h"
>> +#include "svn_checksum.h"
>
> Oof. This shoulda been in there already since svn_checksum_t was in use.
Yeah, it was probably being pulled in by another header somewhere (such as
svn_io.h).
>> ...
>> +++ trunk/subversion/libsvn_fs_base/bdb/dbt.c Sat Oct 18 06:11:59 2008 (r33730)
>> @@ -18,6 +18,8 @@
>> #include <stdlib.h>
>> #include <string.h>
>> #include <apr_pools.h>
>> +#include <apr_md5.h>
>> +#include <apr_sha1.h>
>>
>> #define APU_WANT_DB
>> #include <apu_want.h>
>> @@ -153,3 +155,20 @@ svn_fs_base__str_to_dbt(DBT *dbt, const
>> svn_fs_base__set_dbt(dbt, str, strlen(str));
>> return dbt;
>> }
>> +
>> +DBT *
>> +svn_fs_base__checksum_to_dbt(DBT *dbt, svn_checksum_t *checksum)
>> +{
>> + switch (checksum->kind)
>> + {
>> + case svn_checksum_md5:
>> + svn_fs_base__set_dbt(dbt, checksum->digest, APR_MD5_DIGESTSIZE);
>> + break;
>> +
>> + case svn_checksum_sha1:
>> + svn_fs_base__set_dbt(dbt, checksum->digest, APR_SHA1_DIGESTSIZE);
>> + break;
>> + }
>> +
>> + return dbt;
>> +}
>
> Maybe export the DIGESTSIZE() macro out of libsvn_subr/checksum.c ??
Funny story: We already do! I've updated this function to use
svn_checksum_size() in r33801.
>> ...
>> +++ trunk/subversion/libsvn_fs_base/dag.c Sat Oct 18 06:11:59 2008 (r33730)
>> @@ -33,6 +33,7 @@
>> #include "reps-strings.h"
>> #include "revs-txns.h"
>> #include "id.h"
>> +#include "fsguid.h"
>>
>> #include "util/fs_skels.h"
>>
>> @@ -42,6 +43,7 @@
>> #include "bdb/copies-table.h"
>> #include "bdb/reps-table.h"
>> #include "bdb/strings-table.h"
>> +#include "bdb/checksum-reps-table.h"
>>
>> #include "private/svn_fs_util.h"
>> #include "../libsvn_fs/fs-loader.h"
>> @@ -576,9 +578,15 @@ svn_fs_base__dag_set_proplist(dag_node_t
>> trail_t *trail,
>> apr_pool_t *pool)
>> {
>> + svn_error_t *err;
>> node_revision_t *noderev;
>> - const char *rep_key, *mutable_rep_key;
>> + const char *rep_key, *mutable_rep_key, *dup_rep_key;
>> svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
>> + svn_stream_t *wstream;
>> + apr_size_t len;
>> + skel_t *proplist_skel;
>> + svn_stringbuf_t *raw_proplist_buf;
>> + svn_checksum_t *checksum;
>>
>> /* Sanity check: this node better be mutable! */
>> if (! svn_fs_base__dag_check_mutable(node, txn_id))
>> @@ -595,6 +603,34 @@ svn_fs_base__dag_set_proplist(dag_node_t
>> trail, pool));
>> rep_key = noderev->prop_key;
>>
>> + /* Flatten the proplist into a string, and calculate its md5 hash. */
>> + SVN_ERR(svn_fs_base__unparse_proplist_skel(&proplist_skel,
>> + proplist, pool));
>> + raw_proplist_buf = svn_fs_base__unparse_skel(proplist_skel, pool);
>> + SVN_ERR(svn_checksum(&checksum, svn_checksum_sha1, raw_proplist_buf->data,
>> + raw_proplist_buf->len, pool));
>
> The comment is wrong.
r33801.
>> +
>> + /* If the resulting property list is exactly the same as another
>> + string in the database, just use the previously existing string
>> + and get outta here. */
>> + err = svn_fs_bdb__get_checksum_rep(&dup_rep_key, fs, checksum,
>> + trail, pool);
>> + if (! err)
>> + {
>> + if (noderev->prop_key)
>> + SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, noderev->prop_key,
>> + txn_id, trail, pool));
>> + noderev->prop_key = dup_rep_key;
>> + return svn_fs_bdb__put_node_revision(fs, node->id, noderev,
>> + trail, pool);
>> + }
>> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
>> + {
>> + svn_error_clear(err);
>> + err = SVN_NO_ERROR;
>> + }
>> + SVN_ERR(err);
>
> This last part seems a bit easier to do:
>
> else if (err)
> {
> if (err->apr_err != ...)
> return err;
> svn_error_clear(err);
> }
>
> I mean, the SVN_ERR(err) is only useful for the else-block. The first
> if() means you don't need to check err again. etc... it is simply more
> checking than needed :-P
Good point, again in r33801.
>> ...
>> + if (! err)
>> + {
>> + useless_data_key = noderev->edit_key;
>> + err = svn_fs_base__reserve_fsguid(trail->fs, &data_key_uniquifier,
>> + trail, pool);
>> + }
>> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
>> + {
>> + svn_error_clear(err);
>> + err = SVN_NO_ERROR;
>> + new_data_key = noderev->edit_key;
>> + }
>> + SVN_ERR(err);
>
> Same here.
>
>> ...
>> +++ trunk/subversion/libsvn_fs_base/fs.h Sat Oct 18 06:11:59 2008 (r33730)
>> @@ -41,6 +41,9 @@ extern "C" {
>> back-end's format. */
>> #define SVN_FS_BASE__FORMAT_NUMBER 4
>>
>> +/* Minimum format number that supports representation sharing */
>> +#define SVN_FS_BASE__MIN_REP_SHARING_FORMAT 4
>> +
>> /* Minimum format number that supports the 'miscellaneous' table */
>> #define SVN_FS_BASE__MIN_MISCELLANY_FORMAT 4
>
> Seems like these two MIN constants should be combined into one, for simplicity.
We thought about that over on the BDB side, but decided that overloading the
different features into one macro was a bit confusing. I'd like to keep it as is.
>> ...
>> +++ trunk/subversion/libsvn_fs_base/notes/structure Sat Oct 18 06:11:59 2008 (r33730)
>
> Maybe make FSGUID explicit?
>
> Also: where to note about a format upgrade that will include FSGUID?
> If you're going to migrate all key generation to FSGUID, then you'll
> want to start FSGUID at max(all-next-key-columns). I think if you
> start FSGUID at "0" like the patch is currently doing, you *might*
> close off the door to broader use. Haven't thought it through yet tho.
> But if true, then you're going to want to do some work to get that
> max() before we cut 1.6.
I'll defer this one to Mike.
>> ...
>> +++ trunk/subversion/libsvn_fs_fs/rep-cache.c Sat Oct 18 06:11:59 2008 (r33730, copy of r33729, branches/fs-rep-sharing/subversion/libsvn_fs_fs/rep-cache.c)
>> ...
>> +const char *upgrade_sql[] = { NULL,
>> + "pragma auto_vacuum = 1;"
>> + APR_EOL_STR
>> + "create table rep_cache (hash text not null, "
>> + " revision integer not null, "
>> + " offset integer not null, "
>> + " size integer not null, "
>> + " expanded_size integer not null, "
>> + " reuse_count integer not null); "
>> + APR_EOL_STR
>> + "create unique index i_hash on rep_cache(hash); "
>
> Shouldn't need this custom index if you just mark the hash column as
> the primary key (which would be nice for docco purposes anyways).
>
>> ...
>> +svn_error_t *
>> +svn_fs_fs__get_rep_reference(representation_t **rep,
>> + svn_fs_t *fs,
>> + svn_checksum_t *checksum,
>> + apr_pool_t *pool)
>> +{
>> + fs_fs_data_t *ffd = fs->fsap_data;
>> + svn_boolean_t have_row;
>> + svn_sqlite__stmt_t *stmt;
>> +
>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>> + "select revision, offset, size, expanded_size from rep_cache "
>> + "where hash = :1", pool));
>
> I would recommend preparing this statement when you open the db. Then
> each time this function is called, you can just bind some new values,
> run the query, and finalize the statement. Definitely more efficient.
>
>> ...
>> +svn_error_t *
>> +svn_fs_fs__set_rep_reference(svn_fs_t *fs,
>> + representation_t *rep,
>> + svn_boolean_t reject_dup,
>> + apr_pool_t *pool)
>> +{
>> ...
>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>> + "insert into rep_cache (hash, revision, offset, size, "
>> + "expanded_size, reuse_count) "
>> + "values (:1, :2, :3, :4, :5, 0);", pool));
>
> Likewise.
>
>> ...
>> +svn_error_t *
>> +svn_fs_fs__inc_rep_reuse(svn_fs_t *fs,
>> + representation_t *rep,
>> + apr_pool_t *pool)
>> +{
>> + fs_fs_data_t *ffd = fs->fsap_data;
>> + svn_boolean_t have_row;
>> + svn_sqlite__stmt_t *stmt;
>> +
>> + /* Fetch the current count. */
>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>> + "select reuse_count from rep_cache where hash = :1", pool));
>> ...
>> + /* Update the reuse_count. */
>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>> + "update rep_cache set reuse_count = :1 where hash = :2",
>> + pool));
>
> And two more :-)
Yeah, I can definitely work on fixing up the sqlite usage here. I'll do that in
a separate commit.
-Hyrum
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Joe Swatosh <jo...@gmail.com>.
Hi
On Mon, Nov 3, 2008 at 5:46 AM, Hyrum K. Wright
<hy...@mail.utexas.edu> wrote:
> Joe Swatosh wrote:
>> Hi
>>
>> On Wed, Oct 29, 2008 at 9:54 PM, Joe Swatosh <jo...@gmail.com> wrote:
>>> Howdy Hyrum, kou,
>>>
>>> On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
>>> <hy...@mail.utexas.edu> wrote:
>>>> Joe Swatosh wrote:
>>>>> Hi Hyrum,
>>>>>
>>>>> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>>>>>> Author: hwright
>>>>>> Date: Sat Oct 18 06:11:59 2008
>>>>>> New Revision: 33730
>>>>>>
>>>>>> Log:
>>>>>> Merge the fs-rep-sharing branch to trunk.
>>>>>>
>>>>>> This gives us significant space savings on both bdb and fsfs. See the
>>>>>> branch for individual log messages.
>>>>>>
>>>>> Obviously, this commit broke the windows build, but when it was fixed
>>>>> in r33743 none of the Ruby bindings tests would even run anymore. I
>>>>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>>>>> many of the calls from the bindings into svn. Below is the output
>>>>> along with some comments by me marked by #'s. I hope I'm talking
>>>>> sense. This was run against trunk at r33894.
>>>>>
>>>>>
>>
>> snip
>>
>>>>> I'm just a bystander watching the development of svn proper, but I wonder
>>>>> if there is a better pool to put the clean up of the cache opened by the
>>>>> commit call above? It seems like ideally, it should be released before
>>>>> the invocation of commit completes (like checkout).
>>>> The rep-sharing sqlite database currently closes itself when the pool provided
>>>> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
>>>> doesn't destroy the pool, but instead uses it on a subsequent call to
>>>> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
>>>> and of itself shouldn't be too much of a problem, but I'm wondering if the
>>>> cleanup function we register with APR is getting confused.
>>>>
>>>> Thoughts?
>>>>
>>> Yup, the sqlite database is closed, but for whatever reason
>>> svn_client_commit4 and svn_client_list2 (and others) are just passing
>>> along the pool that was passed to them. I don't think the cleanup
>>> function is confused, I just think the lifetime of the pool we are
>>> passing in is too long. This might not even be problem on non-Windows
>>> OSes that can delete open files.
>>>
>>> The apparent reason that svn_client_checkout3 was working for me was
>>> that it calls svn_client__checkout_internal, which creates a temporary
>>> "session_pool" that is passed to svn_ra_open3 and so the sqlite
>>> database is closed before returning to Ruby.
>
> This part is odd, because the SQLite database should be created in the same pool
> that the filesystem is, and should be closed only when the filesystem is closed.
> If there's a difference in there somewhere, that's a bug.
I haven't really looked at the code much, just did my experiments, so
I couldn't say that it isn't a problem with the bindings keeping the
filesystem open too long. It seems unlikely though since the symptom
we're getting now is that there is an open file that can't be deleted
during test cleanup.
Further, this isn't a likely pattern of use, although it doesn't seem
like it should be prohibited, either. We create repository during
setup, manipulate it during the test then delete it during teardown.
With my patch, I can now pass the first few tests, but eventually one
of the tests does something that causes the cache file to not be
closed, that test errors teardown then the error cascades to all
subsequent tests.
>
>>> I started down the path of creating a patch for the svn_client library
>>> mimicking that pattern of creating session_pools, but when I got to
>>> svn_client_open_ra_session, I started doubting the approach. It seems
>>> to me that if we are returning an RA session object that perhaps the
>>
>> Trying to make this a little more concrete, I've attached the patch in
>> progress. The more I think about it the less I like it. It seems
>> like there may be synchronization issues with having different
>> functions accessing the cache with different file handles.
>
> We've got some validation to do, to be sure (and I believe that's part of the
> notes that Dave made about FSFS in TODO-1.6). However, SQLite handles the
> multiple access issues for us. We're not working with different file handles,
> we're working with different database handles, and the SQLite library ensures
> that we can do that.
>
> Perchance, are experiencing this problem on any kind of network file system?
>
No, its all on a disk.
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include
subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr
subvers
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Joe Swatosh wrote:
> Hi
>
> On Wed, Oct 29, 2008 at 9:54 PM, Joe Swatosh <jo...@gmail.com> wrote:
>> Howdy Hyrum, kou,
>>
>> On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
>> <hy...@mail.utexas.edu> wrote:
>>> Joe Swatosh wrote:
>>>> Hi Hyrum,
>>>>
>>>> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>>>>> Author: hwright
>>>>> Date: Sat Oct 18 06:11:59 2008
>>>>> New Revision: 33730
>>>>>
>>>>> Log:
>>>>> Merge the fs-rep-sharing branch to trunk.
>>>>>
>>>>> This gives us significant space savings on both bdb and fsfs. See the
>>>>> branch for individual log messages.
>>>>>
>>>> Obviously, this commit broke the windows build, but when it was fixed
>>>> in r33743 none of the Ruby bindings tests would even run anymore. I
>>>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>>>> many of the calls from the bindings into svn. Below is the output
>>>> along with some comments by me marked by #'s. I hope I'm talking
>>>> sense. This was run against trunk at r33894.
>>>>
>>>>
>
> snip
>
>>>> I'm just a bystander watching the development of svn proper, but I wonder
>>>> if there is a better pool to put the clean up of the cache opened by the
>>>> commit call above? It seems like ideally, it should be released before
>>>> the invocation of commit completes (like checkout).
>>> The rep-sharing sqlite database currently closes itself when the pool provided
>>> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
>>> doesn't destroy the pool, but instead uses it on a subsequent call to
>>> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
>>> and of itself shouldn't be too much of a problem, but I'm wondering if the
>>> cleanup function we register with APR is getting confused.
>>>
>>> Thoughts?
>>>
>> Yup, the sqlite database is closed, but for whatever reason
>> svn_client_commit4 and svn_client_list2 (and others) are just passing
>> along the pool that was passed to them. I don't think the cleanup
>> function is confused, I just think the lifetime of the pool we are
>> passing in is too long. This might not even be problem on non-Windows
>> OSes that can delete open files.
>>
>> The apparent reason that svn_client_checkout3 was working for me was
>> that it calls svn_client__checkout_internal, which creates a temporary
>> "session_pool" that is passed to svn_ra_open3 and so the sqlite
>> database is closed before returning to Ruby.
This part is odd, because the SQLite database should be created in the same pool
that the filesystem is, and should be closed only when the filesystem is closed.
If there's a difference in there somewhere, that's a bug.
>> I started down the path of creating a patch for the svn_client library
>> mimicking that pattern of creating session_pools, but when I got to
>> svn_client_open_ra_session, I started doubting the approach. It seems
>> to me that if we are returning an RA session object that perhaps the
>
> Trying to make this a little more concrete, I've attached the patch in
> progress. The more I think about it the less I like it. It seems
> like there may be synchronization issues with having different
> functions accessing the cache with different file handles.
We've got some validation to do, to be sure (and I believe that's part of the
notes that Dave made about FSFS in TODO-1.6). However, SQLite handles the
multiple access issues for us. We're not working with different file handles,
we're working with different database handles, and the SQLite library ensures
that we can do that.
Perchance, are experiencing this problem on any kind of network file system?
-Hyrum
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Joe Swatosh <jo...@gmail.com>.
Hi
On Wed, Oct 29, 2008 at 9:54 PM, Joe Swatosh <jo...@gmail.com> wrote:
> Howdy Hyrum, kou,
>
> On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
> <hy...@mail.utexas.edu> wrote:
>> Joe Swatosh wrote:
>>> Hi Hyrum,
>>>
>>> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>>>> Author: hwright
>>>> Date: Sat Oct 18 06:11:59 2008
>>>> New Revision: 33730
>>>>
>>>> Log:
>>>> Merge the fs-rep-sharing branch to trunk.
>>>>
>>>> This gives us significant space savings on both bdb and fsfs. See the
>>>> branch for individual log messages.
>>>>
>>>
>>> Obviously, this commit broke the windows build, but when it was fixed
>>> in r33743 none of the Ruby bindings tests would even run anymore. I
>>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>>> many of the calls from the bindings into svn. Below is the output
>>> along with some comments by me marked by #'s. I hope I'm talking
>>> sense. This was run against trunk at r33894.
>>>
>>>
snip
>>>
>>> I'm just a bystander watching the development of svn proper, but I wonder
>>> if there is a better pool to put the clean up of the cache opened by the
>>> commit call above? It seems like ideally, it should be released before
>>> the invocation of commit completes (like checkout).
>>
>> The rep-sharing sqlite database currently closes itself when the pool provided
>> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
>> doesn't destroy the pool, but instead uses it on a subsequent call to
>> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
>> and of itself shouldn't be too much of a problem, but I'm wondering if the
>> cleanup function we register with APR is getting confused.
>>
>> Thoughts?
>>
>
> Yup, the sqlite database is closed, but for whatever reason
> svn_client_commit4 and svn_client_list2 (and others) are just passing
> along the pool that was passed to them. I don't think the cleanup
> function is confused, I just think the lifetime of the pool we are
> passing in is too long. This might not even be problem on non-Windows
> OSes that can delete open files.
>
> The apparent reason that svn_client_checkout3 was working for me was
> that it calls svn_client__checkout_internal, which creates a temporary
> "session_pool" that is passed to svn_ra_open3 and so the sqlite
> database is closed before returning to Ruby.
>
> I started down the path of creating a patch for the svn_client library
> mimicking that pattern of creating session_pools, but when I got to
> svn_client_open_ra_session, I started doubting the approach. It seems
> to me that if we are returning an RA session object that perhaps the
Trying to make this a little more concrete, I've attached the patch in
progress. The more I think about it the less I like it. It seems
like there may be synchronization issues with having different
functions accessing the cache with different file handles.
--
Joe
> handle to the cache ought to stick around for the lifetime of the
> session object. Or if that isn't correct maybe the bindings should
> create a pool for the duration? I looked at this a little, but I'm
> not sure how to accomplish that. Or maybe I should just continue
> creating session pools and post the patch?
>
> More questions than answers, I'm afraid.
> --
> Joe
>
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Joe Swatosh <jo...@gmail.com>.
Howdy Hyrum, kou,
On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
<hy...@mail.utexas.edu> wrote:
> Joe Swatosh wrote:
>> Hi Hyrum,
>>
>> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>>> Author: hwright
>>> Date: Sat Oct 18 06:11:59 2008
>>> New Revision: 33730
>>>
>>> Log:
>>> Merge the fs-rep-sharing branch to trunk.
>>>
>>> This gives us significant space savings on both bdb and fsfs. See the
>>> branch for individual log messages.
>>>
>>
>> Obviously, this commit broke the windows build, but when it was fixed
>> in r33743 none of the Ruby bindings tests would even run anymore. I
>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>> many of the calls from the bindings into svn. Below is the output
>> along with some comments by me marked by #'s. I hope I'm talking
>> sense. This was run against trunk at r33894.
>>
>>
>> # A bunch of initialization
>> Svn::Ext::Core.svn_nls_init start
>> Svn::Ext::Core.svn_nls_init end
>> Svn::Ext::Core.svn_default_charset start
>> Svn::Ext::Core.svn_default_charset end
>> Svn::Ext::Core.svn_locale_charset start
>> Svn::Ext::Core.svn_locale_charset end
>> Svn::Ext::Fs.svn_fs_initialize start
>> Svn::Ext::Fs.svn_fs_initialize end
>> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack start
>> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack end
>> Svn::Ext::Ra.svn_ra_initialize start
>> Svn::Ext::Ra.svn_ra_initialize end Loaded suite test_info.rb
>> Started
>> test_info(SvnInfoTest):
>> #
>> # Setting up for the test creates a repo (We weren't closing it after the
>> # create, but we do now).
>> #
>> Svn::Ext::Repos.svn_repos_create start sqlite3_open_v2(2ec9780)
>> Svn::Ext::Repos.svn_repos_create end
>> x.close start sqlite3_close(2ec9780)
>> x.close end
>> #
>> # Still in setup we open the repository and keep a reference for later
>> #
>> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2ec9780)
>> Svn::Ext::Repos.svn_repos_open end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook start
>> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook end
>> Svn::Ext::Core.svn_config_ensure start
>> Svn::Ext::Core.svn_config_ensure end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Client.svn_client_set_notify_func2 start
>> Svn::Ext::Client.svn_client_set_notify_func2 end
>> Svn::Ext::Client.svn_client_set_cancel_func start
>> Svn::Ext::Client.svn_client_set_cancel_func end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> #
>> # It looks like doing the checkout opens and closes the cache twice. (Reusing
>> # the same address).
>> #
>> Svn::Ext::Client.svn_client_checkout3
>> start sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8) sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8)
>> Svn::Ext::Client.svn_client_checkout3 end
>> Svn::Ext::Repos.svn_repos_svnserve_conf start
>> Svn::Ext::Repos.svn_repos_svnserve_conf end
>> Svn::Ext::Repos.svn_repos_conf_dir start
>> Svn::Ext::Repos.svn_repos_conf_dir end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Client.svn_client_set_notify_func2 start
>> Svn::Ext::Client.svn_client_set_notify_func2 end
>> Svn::Ext::Client.svn_client_set_cancel_func start
>> Svn::Ext::Client.svn_client_set_cancel_func end
>> Svn::Ext::Client.svn_client_set_log_msg_func3 start
>> Svn::Ext::Client.svn_client_set_log_msg_func3 end
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
>> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Core.svn_auth_set_parameter start
>> Svn::Ext::Core.svn_auth_set_parameter end
>> Svn::Ext::Client.svn_client_add3 start
>> Svn::Ext::Client.svn_client_add3 end
>> #
>> # commit is opening the cache, but not closing it. I think this is what is
>> # causing the test failures, more below...
>> #
>> Svn::Ext::Client.svn_client_commit4 start sqlite3_open_v2(2eca1b8)
>> Svn::Ext::Client.svn_client_commit4 end
>> #
>> # Okay, another place that the bindings opened a repo, but didn't close
>> # it that is now better in my wc
>> #
>> make_info start
>> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2f67750)
>> Svn::Ext::Repos.svn_repos_open end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_dir_delta start
>> Svn::Ext::Repos.svn_repos_dir_delta end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_dir_delta start
>> Svn::Ext::Fs.svn_fs_copied_from start
>> Svn::Ext::Fs.svn_fs_copied_from end
>> Svn::Ext::Repos.svn_repos_dir_delta end
>> Svn::Ext::Repos.svn_repos_fs_wrapper start
>> Svn::Ext::Repos.svn_repos_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Repos.svn_repos_node_editor start
>> Svn::Ext::Repos.svn_repos_node_editor end
>> Svn::Ext::Repos.svn_repos_replay2 start
>> Svn::Ext::Repos.svn_repos_replay2 end
>> Svn::Ext::Repos.svn_repos_node_from_baton start
>> Svn::Ext::Repos.svn_repos_node_from_baton end
>> Svn::Ext::Fs.svn_fs_revision_root start
>> Svn::Ext::Fs.svn_fs_revision_root end
>> Svn::Ext::Fs.svn_fs_node_prop start
>> Svn::Ext::Fs.svn_fs_node_prop end
>> Svn::Ext::Fs.svn_fs_revision_root_revision start
>> Svn::Ext::Fs.svn_fs_revision_root_revision end
>> Svn::Ext::Fs.svn_fs_revision_root_revision start
>> Svn::Ext::Fs.svn_fs_revision_root_revision end
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
>> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
>> Svn::Ext::Fs.svn_fs_revision_prop start
>> Svn::Ext::Fs.svn_fs_revision_prop end
>> Svn::Ext::Core.svn_parse_date start
>> Svn::Ext::Core.svn_parse_date end
>> Svn::Ext::Fs.svn_fs_file_contents start
>> Svn::Ext::Fs.svn_fs_file_contents end
>> Svn::Ext::Core.svn_stream_read start
>> Svn::Ext::Core.svn_stream_read end
>> Svn::Ext::Core.svn_stream_close start
>> Svn::Ext::Core.svn_stream_close end
>> Svn::Ext::Core.svn_diff_file_diff_2 start
>> Svn::Ext::Core.svn_diff_file_diff_2 end
>> Svn::Ext::Fs.svn_fs_file_contents start
>> Svn::Ext::Fs.svn_fs_file_contents end
>> Svn::Ext::Core.svn_stream_read start
>> Svn::Ext::Core.svn_stream_read end
>> Svn::Ext::Core.svn_stream_close start
>> #
>> # closing repo opened by make_info() implementation....
>> #
>> Svn::Ext::Core.svn_stream_close end sqlite3_close(2f67750)
>> make_info end
>> Svn::Ext::Core.svn_time_from_cstring start
>> Svn::Ext::Core.svn_time_from_cstring end
>> #
>> # test teardown, closing the repo opened in the setup
>> #
>> @repos.close start sqlite3_close(2ec9780)
>> @repos.close end
>> #
>> # ohoh, can't delete the repo because the cache is still open.
>> #
>> Svn::Ext::Repos.svn_repos_delete start E
>>
>> Finished in 13.965 seconds.
>>
>> 1) Error:
>> test_info(SvnInfoTest):
>> Svn::Error::SvnError:
>> D:\SVN\src-trunk\subversion\libsvn_subr\io.c:1714: 720032: Can't
>> remove file 'repos\db\rep-cache.db': The process cannot access the
>> file because it is being used by another process.
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in
>> `svn_repos_delete'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in `delete'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:114:in
>> `teardown_repository'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:48:in
>> `teardown_basic'
>> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test\test_info.rb:13:in
>> `teardown'
>>
>> 1 tests, 4 assertions, 0 failures, 1 errors
>> #
>> # Ah, process wind down or GC is going to release that last link to the
>> # sqlite cache.
>> #
>> sqlite3_close(2eca1b8)
>>
>> ********************************************************************************
>>
>> I'm just a bystander watching the development of svn proper, but I wonder
>> if there is a better pool to put the clean up of the cache opened by the
>> commit call above? It seems like ideally, it should be released before
>> the invocation of commit completes (like checkout).
>
> The rep-sharing sqlite database currently closes itself when the pool provided
> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
> doesn't destroy the pool, but instead uses it on a subsequent call to
> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
> and of itself shouldn't be too much of a problem, but I'm wondering if the
> cleanup function we register with APR is getting confused.
>
> Thoughts?
>
Yup, the sqlite database is closed, but for whatever reason
svn_client_commit4 and svn_client_list2 (and others) are just passing
along the pool that was passed to them. I don't think the cleanup
function is confused, I just think the lifetime of the pool we are
passing in is too long. This might not even be problem on non-Windows
OSes that can delete open files.
The apparent reason that svn_client_checkout3 was working for me was
that it calls svn_client__checkout_internal, which creates a temporary
"session_pool" that is passed to svn_ra_open3 and so the sqlite
database is closed before returning to Ruby.
I started down the path of creating a patch for the svn_client library
mimicking that pattern of creating session_pools, but when I got to
svn_client_open_ra_session, I started doubting the approach. It seems
to me that if we are returning an RA session object that perhaps the
handle to the cache ought to stick around for the lifetime of the
session object. Or if that isn't correct maybe the bindings should
create a pool for the duration? I looked at this a little, but I'm
not sure how to accomplish that. Or maybe I should just continue
creating session pools and post the patch?
More questions than answers, I'm afraid.
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include
subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr
subvers
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Joe Swatosh wrote:
> Hi Hyrum,
>
> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>> Author: hwright
>> Date: Sat Oct 18 06:11:59 2008
>> New Revision: 33730
>>
>> Log:
>> Merge the fs-rep-sharing branch to trunk.
>>
>> This gives us significant space savings on both bdb and fsfs. See the
>> branch for individual log messages.
>>
>
> Obviously, this commit broke the windows build, but when it was fixed
> in r33743 none of the Ruby bindings tests would even run anymore. I
> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
> many of the calls from the bindings into svn. Below is the output
> along with some comments by me marked by #'s. I hope I'm talking
> sense. This was run against trunk at r33894.
>
>
> # A bunch of initialization
> Svn::Ext::Core.svn_nls_init start
> Svn::Ext::Core.svn_nls_init end
> Svn::Ext::Core.svn_default_charset start
> Svn::Ext::Core.svn_default_charset end
> Svn::Ext::Core.svn_locale_charset start
> Svn::Ext::Core.svn_locale_charset end
> Svn::Ext::Fs.svn_fs_initialize start
> Svn::Ext::Fs.svn_fs_initialize end
> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack start
> Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack end
> Svn::Ext::Ra.svn_ra_initialize start
> Svn::Ext::Ra.svn_ra_initialize end Loaded suite test_info.rb
> Started
> test_info(SvnInfoTest):
> #
> # Setting up for the test creates a repo (We weren't closing it after the
> # create, but we do now).
> #
> Svn::Ext::Repos.svn_repos_create start sqlite3_open_v2(2ec9780)
> Svn::Ext::Repos.svn_repos_create end
> x.close start sqlite3_close(2ec9780)
> x.close end
> #
> # Still in setup we open the repository and keep a reference for later
> #
> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2ec9780)
> Svn::Ext::Repos.svn_repos_open end
> Svn::Ext::Repos.svn_repos_fs_wrapper start
> Svn::Ext::Repos.svn_repos_fs_wrapper end
> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook start
> Svn::Ext::Repos.svn_repos_pre_revprop_change_hook end
> Svn::Ext::Core.svn_config_ensure start
> Svn::Ext::Core.svn_config_ensure end
> Svn::Ext::Client.svn_client_set_log_msg_func3 start
> Svn::Ext::Client.svn_client_set_log_msg_func3 end
> Svn::Ext::Client.svn_client_set_notify_func2 start
> Svn::Ext::Client.svn_client_set_notify_func2 end
> Svn::Ext::Client.svn_client_set_cancel_func start
> Svn::Ext::Client.svn_client_set_cancel_func end
> Svn::Ext::Client.svn_client_set_log_msg_func3 start
> Svn::Ext::Client.svn_client_set_log_msg_func3 end
> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
> Svn::Ext::Core.svn_auth_set_parameter start
> Svn::Ext::Core.svn_auth_set_parameter end
> Svn::Ext::Core.svn_auth_set_parameter start
> Svn::Ext::Core.svn_auth_set_parameter end
> #
> # It looks like doing the checkout opens and closes the cache twice. (Reusing
> # the same address).
> #
> Svn::Ext::Client.svn_client_checkout3
> start sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8) sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8)
> Svn::Ext::Client.svn_client_checkout3 end
> Svn::Ext::Repos.svn_repos_svnserve_conf start
> Svn::Ext::Repos.svn_repos_svnserve_conf end
> Svn::Ext::Repos.svn_repos_conf_dir start
> Svn::Ext::Repos.svn_repos_conf_dir end
> Svn::Ext::Client.svn_client_set_log_msg_func3 start
> Svn::Ext::Client.svn_client_set_log_msg_func3 end
> Svn::Ext::Client.svn_client_set_notify_func2 start
> Svn::Ext::Client.svn_client_set_notify_func2 end
> Svn::Ext::Client.svn_client_set_cancel_func start
> Svn::Ext::Client.svn_client_set_cancel_func end
> Svn::Ext::Client.svn_client_set_log_msg_func3 start
> Svn::Ext::Client.svn_client_set_log_msg_func3 end
> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
> Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
> Svn::Ext::Core.svn_auth_set_parameter start
> Svn::Ext::Core.svn_auth_set_parameter end
> Svn::Ext::Core.svn_auth_set_parameter start
> Svn::Ext::Core.svn_auth_set_parameter end
> Svn::Ext::Client.svn_client_add3 start
> Svn::Ext::Client.svn_client_add3 end
> #
> # commit is opening the cache, but not closing it. I think this is what is
> # causing the test failures, more below...
> #
> Svn::Ext::Client.svn_client_commit4 start sqlite3_open_v2(2eca1b8)
> Svn::Ext::Client.svn_client_commit4 end
> #
> # Okay, another place that the bindings opened a repo, but didn't close
> # it that is now better in my wc
> #
> make_info start
> Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2f67750)
> Svn::Ext::Repos.svn_repos_open end
> Svn::Ext::Repos.svn_repos_fs_wrapper start
> Svn::Ext::Repos.svn_repos_fs_wrapper end
> Svn::Ext::Fs.svn_fs_revision_root start
> Svn::Ext::Fs.svn_fs_revision_root end
> Svn::Ext::Fs.svn_fs_revision_prop start
> Svn::Ext::Fs.svn_fs_revision_prop end
> Svn::Ext::Fs.svn_fs_revision_prop start
> Svn::Ext::Fs.svn_fs_revision_prop end
> Svn::Ext::Core.svn_parse_date start
> Svn::Ext::Core.svn_parse_date end
> Svn::Ext::Fs.svn_fs_revision_prop start
> Svn::Ext::Fs.svn_fs_revision_prop end
> Svn::Ext::Fs.svn_fs_revision_root start
> Svn::Ext::Fs.svn_fs_revision_root end
> Svn::Ext::Repos.svn_repos_dir_delta start
> Svn::Ext::Repos.svn_repos_dir_delta end
> Svn::Ext::Fs.svn_fs_revision_root start
> Svn::Ext::Fs.svn_fs_revision_root end
> Svn::Ext::Repos.svn_repos_dir_delta start
> Svn::Ext::Fs.svn_fs_copied_from start
> Svn::Ext::Fs.svn_fs_copied_from end
> Svn::Ext::Repos.svn_repos_dir_delta end
> Svn::Ext::Repos.svn_repos_fs_wrapper start
> Svn::Ext::Repos.svn_repos_fs_wrapper end
> Svn::Ext::Fs.svn_fs_revision_root start
> Svn::Ext::Fs.svn_fs_revision_root end
> Svn::Ext::Repos.svn_repos_node_editor start
> Svn::Ext::Repos.svn_repos_node_editor end
> Svn::Ext::Repos.svn_repos_replay2 start
> Svn::Ext::Repos.svn_repos_replay2 end
> Svn::Ext::Repos.svn_repos_node_from_baton start
> Svn::Ext::Repos.svn_repos_node_from_baton end
> Svn::Ext::Fs.svn_fs_revision_root start
> Svn::Ext::Fs.svn_fs_revision_root end
> Svn::Ext::Fs.svn_fs_node_prop start
> Svn::Ext::Fs.svn_fs_node_prop end
> Svn::Ext::Fs.svn_fs_revision_root_revision start
> Svn::Ext::Fs.svn_fs_revision_root_revision end
> Svn::Ext::Fs.svn_fs_revision_root_revision start
> Svn::Ext::Fs.svn_fs_revision_root_revision end
> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
> Svn::Ext::Fs.svn_fs_revision_prop start
> Svn::Ext::Fs.svn_fs_revision_prop end
> Svn::Ext::Core.svn_parse_date start
> Svn::Ext::Core.svn_parse_date end
> Svn::Ext::Fs.svn_fs_root_fs_wrapper start
> Svn::Ext::Fs.svn_fs_root_fs_wrapper end
> Svn::Ext::Fs.svn_fs_revision_prop start
> Svn::Ext::Fs.svn_fs_revision_prop end
> Svn::Ext::Core.svn_parse_date start
> Svn::Ext::Core.svn_parse_date end
> Svn::Ext::Fs.svn_fs_file_contents start
> Svn::Ext::Fs.svn_fs_file_contents end
> Svn::Ext::Core.svn_stream_read start
> Svn::Ext::Core.svn_stream_read end
> Svn::Ext::Core.svn_stream_close start
> Svn::Ext::Core.svn_stream_close end
> Svn::Ext::Core.svn_diff_file_diff_2 start
> Svn::Ext::Core.svn_diff_file_diff_2 end
> Svn::Ext::Fs.svn_fs_file_contents start
> Svn::Ext::Fs.svn_fs_file_contents end
> Svn::Ext::Core.svn_stream_read start
> Svn::Ext::Core.svn_stream_read end
> Svn::Ext::Core.svn_stream_close start
> #
> # closing repo opened by make_info() implementation....
> #
> Svn::Ext::Core.svn_stream_close end sqlite3_close(2f67750)
> make_info end
> Svn::Ext::Core.svn_time_from_cstring start
> Svn::Ext::Core.svn_time_from_cstring end
> #
> # test teardown, closing the repo opened in the setup
> #
> @repos.close start sqlite3_close(2ec9780)
> @repos.close end
> #
> # ohoh, can't delete the repo because the cache is still open.
> #
> Svn::Ext::Repos.svn_repos_delete start E
>
> Finished in 13.965 seconds.
>
> 1) Error:
> test_info(SvnInfoTest):
> Svn::Error::SvnError:
> D:\SVN\src-trunk\subversion\libsvn_subr\io.c:1714: 720032: Can't
> remove file 'repos\db\rep-cache.db': The process cannot access the
> file because it is being used by another process.
> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in
> `svn_repos_delete'
> D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in `delete'
> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:114:in
> `teardown_repository'
> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:48:in
> `teardown_basic'
> D:/SVN/src-trunk/subversion/bindings/swig/ruby/test\test_info.rb:13:in
> `teardown'
>
> 1 tests, 4 assertions, 0 failures, 1 errors
> #
> # Ah, process wind down or GC is going to release that last link to the
> # sqlite cache.
> #
> sqlite3_close(2eca1b8)
>
> ********************************************************************************
>
> I'm just a bystander watching the development of svn proper, but I wonder
> if there is a better pool to put the clean up of the cache opened by the
> commit call above? It seems like ideally, it should be released before
> the invocation of commit completes (like checkout).
The rep-sharing sqlite database currently closes itself when the pool provided
to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
doesn't destroy the pool, but instead uses it on a subsequent call to
svn_fs_open(), then we'll have two open handles to the sqlite database. That in
and of itself shouldn't be too much of a problem, but I'm wondering if the
cleanup function we register with APR is getting confused.
Thoughts?
-Hyrum
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Joe Swatosh <jo...@gmail.com>.
Hi Hyrum,
On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
> Author: hwright
> Date: Sat Oct 18 06:11:59 2008
> New Revision: 33730
>
> Log:
> Merge the fs-rep-sharing branch to trunk.
>
> This gives us significant space savings on both bdb and fsfs. See the
> branch for individual log messages.
>
Obviously, this commit broke the windows build, but when it was fixed
in r33743 none of the Ruby bindings tests would even run anymore. I
instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
many of the calls from the bindings into svn. Below is the output
along with some comments by me marked by #'s. I hope I'm talking
sense. This was run against trunk at r33894.
# A bunch of initialization
Svn::Ext::Core.svn_nls_init start
Svn::Ext::Core.svn_nls_init end
Svn::Ext::Core.svn_default_charset start
Svn::Ext::Core.svn_default_charset end
Svn::Ext::Core.svn_locale_charset start
Svn::Ext::Core.svn_locale_charset end
Svn::Ext::Fs.svn_fs_initialize start
Svn::Ext::Fs.svn_fs_initialize end
Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack start
Svn::Ext::Wc.svn_wc_swig_init_asp_dot_net_hack end
Svn::Ext::Ra.svn_ra_initialize start
Svn::Ext::Ra.svn_ra_initialize end Loaded suite test_info.rb
Started
test_info(SvnInfoTest):
#
# Setting up for the test creates a repo (We weren't closing it after the
# create, but we do now).
#
Svn::Ext::Repos.svn_repos_create start sqlite3_open_v2(2ec9780)
Svn::Ext::Repos.svn_repos_create end
x.close start sqlite3_close(2ec9780)
x.close end
#
# Still in setup we open the repository and keep a reference for later
#
Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2ec9780)
Svn::Ext::Repos.svn_repos_open end
Svn::Ext::Repos.svn_repos_fs_wrapper start
Svn::Ext::Repos.svn_repos_fs_wrapper end
Svn::Ext::Repos.svn_repos_pre_revprop_change_hook start
Svn::Ext::Repos.svn_repos_pre_revprop_change_hook end
Svn::Ext::Core.svn_config_ensure start
Svn::Ext::Core.svn_config_ensure end
Svn::Ext::Client.svn_client_set_log_msg_func3 start
Svn::Ext::Client.svn_client_set_log_msg_func3 end
Svn::Ext::Client.svn_client_set_notify_func2 start
Svn::Ext::Client.svn_client_set_notify_func2 end
Svn::Ext::Client.svn_client_set_cancel_func start
Svn::Ext::Client.svn_client_set_cancel_func end
Svn::Ext::Client.svn_client_set_log_msg_func3 start
Svn::Ext::Client.svn_client_set_log_msg_func3 end
Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
Svn::Ext::Core.svn_auth_set_parameter start
Svn::Ext::Core.svn_auth_set_parameter end
Svn::Ext::Core.svn_auth_set_parameter start
Svn::Ext::Core.svn_auth_set_parameter end
#
# It looks like doing the checkout opens and closes the cache twice. (Reusing
# the same address).
#
Svn::Ext::Client.svn_client_checkout3
start sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8) sqlite3_open_v2(2eca1b8) sqlite3_close(2eca1b8)
Svn::Ext::Client.svn_client_checkout3 end
Svn::Ext::Repos.svn_repos_svnserve_conf start
Svn::Ext::Repos.svn_repos_svnserve_conf end
Svn::Ext::Repos.svn_repos_conf_dir start
Svn::Ext::Repos.svn_repos_conf_dir end
Svn::Ext::Client.svn_client_set_log_msg_func3 start
Svn::Ext::Client.svn_client_set_log_msg_func3 end
Svn::Ext::Client.svn_client_set_notify_func2 start
Svn::Ext::Client.svn_client_set_notify_func2 end
Svn::Ext::Client.svn_client_set_cancel_func start
Svn::Ext::Client.svn_client_set_cancel_func end
Svn::Ext::Client.svn_client_set_log_msg_func3 start
Svn::Ext::Client.svn_client_set_log_msg_func3 end
Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider start
Svn::Ext::Core.svn_swig_rb_auth_get_username_prompt_provider end
Svn::Ext::Core.svn_auth_set_parameter start
Svn::Ext::Core.svn_auth_set_parameter end
Svn::Ext::Core.svn_auth_set_parameter start
Svn::Ext::Core.svn_auth_set_parameter end
Svn::Ext::Client.svn_client_add3 start
Svn::Ext::Client.svn_client_add3 end
#
# commit is opening the cache, but not closing it. I think this is what is
# causing the test failures, more below...
#
Svn::Ext::Client.svn_client_commit4 start sqlite3_open_v2(2eca1b8)
Svn::Ext::Client.svn_client_commit4 end
#
# Okay, another place that the bindings opened a repo, but didn't close
# it that is now better in my wc
#
make_info start
Svn::Ext::Repos.svn_repos_open start sqlite3_open_v2(2f67750)
Svn::Ext::Repos.svn_repos_open end
Svn::Ext::Repos.svn_repos_fs_wrapper start
Svn::Ext::Repos.svn_repos_fs_wrapper end
Svn::Ext::Fs.svn_fs_revision_root start
Svn::Ext::Fs.svn_fs_revision_root end
Svn::Ext::Fs.svn_fs_revision_prop start
Svn::Ext::Fs.svn_fs_revision_prop end
Svn::Ext::Fs.svn_fs_revision_prop start
Svn::Ext::Fs.svn_fs_revision_prop end
Svn::Ext::Core.svn_parse_date start
Svn::Ext::Core.svn_parse_date end
Svn::Ext::Fs.svn_fs_revision_prop start
Svn::Ext::Fs.svn_fs_revision_prop end
Svn::Ext::Fs.svn_fs_revision_root start
Svn::Ext::Fs.svn_fs_revision_root end
Svn::Ext::Repos.svn_repos_dir_delta start
Svn::Ext::Repos.svn_repos_dir_delta end
Svn::Ext::Fs.svn_fs_revision_root start
Svn::Ext::Fs.svn_fs_revision_root end
Svn::Ext::Repos.svn_repos_dir_delta start
Svn::Ext::Fs.svn_fs_copied_from start
Svn::Ext::Fs.svn_fs_copied_from end
Svn::Ext::Repos.svn_repos_dir_delta end
Svn::Ext::Repos.svn_repos_fs_wrapper start
Svn::Ext::Repos.svn_repos_fs_wrapper end
Svn::Ext::Fs.svn_fs_revision_root start
Svn::Ext::Fs.svn_fs_revision_root end
Svn::Ext::Repos.svn_repos_node_editor start
Svn::Ext::Repos.svn_repos_node_editor end
Svn::Ext::Repos.svn_repos_replay2 start
Svn::Ext::Repos.svn_repos_replay2 end
Svn::Ext::Repos.svn_repos_node_from_baton start
Svn::Ext::Repos.svn_repos_node_from_baton end
Svn::Ext::Fs.svn_fs_revision_root start
Svn::Ext::Fs.svn_fs_revision_root end
Svn::Ext::Fs.svn_fs_node_prop start
Svn::Ext::Fs.svn_fs_node_prop end
Svn::Ext::Fs.svn_fs_revision_root_revision start
Svn::Ext::Fs.svn_fs_revision_root_revision end
Svn::Ext::Fs.svn_fs_revision_root_revision start
Svn::Ext::Fs.svn_fs_revision_root_revision end
Svn::Ext::Fs.svn_fs_root_fs_wrapper start
Svn::Ext::Fs.svn_fs_root_fs_wrapper end
Svn::Ext::Fs.svn_fs_revision_prop start
Svn::Ext::Fs.svn_fs_revision_prop end
Svn::Ext::Core.svn_parse_date start
Svn::Ext::Core.svn_parse_date end
Svn::Ext::Fs.svn_fs_root_fs_wrapper start
Svn::Ext::Fs.svn_fs_root_fs_wrapper end
Svn::Ext::Fs.svn_fs_revision_prop start
Svn::Ext::Fs.svn_fs_revision_prop end
Svn::Ext::Core.svn_parse_date start
Svn::Ext::Core.svn_parse_date end
Svn::Ext::Fs.svn_fs_file_contents start
Svn::Ext::Fs.svn_fs_file_contents end
Svn::Ext::Core.svn_stream_read start
Svn::Ext::Core.svn_stream_read end
Svn::Ext::Core.svn_stream_close start
Svn::Ext::Core.svn_stream_close end
Svn::Ext::Core.svn_diff_file_diff_2 start
Svn::Ext::Core.svn_diff_file_diff_2 end
Svn::Ext::Fs.svn_fs_file_contents start
Svn::Ext::Fs.svn_fs_file_contents end
Svn::Ext::Core.svn_stream_read start
Svn::Ext::Core.svn_stream_read end
Svn::Ext::Core.svn_stream_close start
#
# closing repo opened by make_info() implementation....
#
Svn::Ext::Core.svn_stream_close end sqlite3_close(2f67750)
make_info end
Svn::Ext::Core.svn_time_from_cstring start
Svn::Ext::Core.svn_time_from_cstring end
#
# test teardown, closing the repo opened in the setup
#
@repos.close start sqlite3_close(2ec9780)
@repos.close end
#
# ohoh, can't delete the repo because the cache is still open.
#
Svn::Ext::Repos.svn_repos_delete start E
Finished in 13.965 seconds.
1) Error:
test_info(SvnInfoTest):
Svn::Error::SvnError:
D:\SVN\src-trunk\subversion\libsvn_subr\io.c:1714: 720032: Can't
remove file 'repos\db\rep-cache.db': The process cannot access the
file because it is being used by another process.
D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in
`svn_repos_delete'
D:/SVN/src-trunk/subversion/bindings/swig/ruby/svn/util.rb:88:in `delete'
D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:114:in
`teardown_repository'
D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/util.rb:48:in
`teardown_basic'
D:/SVN/src-trunk/subversion/bindings/swig/ruby/test\test_info.rb:13:in
`teardown'
1 tests, 4 assertions, 0 failures, 1 errors
#
# Ah, process wind down or GC is going to release that last link to the
# sqlite cache.
#
sqlite3_close(2eca1b8)
********************************************************************************
I'm just a bystander watching the development of svn proper, but I wonder
if there is a better pool to put the clean up of the cache opened by the
commit call above? It seems like ideally, it should be released before
the invocation of commit completes (like checkout).
Thanks,
--
Joe Swatosh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include
subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr
subvers
Posted by Branko Čibej <br...@xbc.nu>.
Guys, could you please try to quote only relevant parts in replies
please ... digging through 50+ lines of quotes to find one sentence of
reply is not my idea of fun.
Bert Huijben wrote:
>
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: maandag 20 oktober 2008 12:51
>> To: dev@subversion.tigris.org; hwright@tigris.org
>> Subject: Re: svn commit: r33730 - in trunk: . notes/tree-conflicts
>> subversion/include subversion/libsvn_fs_base
>> subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
>> subversion/libsvn_fs_base/util subversion/libsvn_fs_fs
>> subversion/libsvn_subr subvers
>>
>> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
>>
>>> ...
>>>
>>> +++ trunk/subversion/include/svn_error_codes.h Sat Oct 18 06:11:59
>>>
>> 2008 (r33730)
>>
>>> @@ -673,6 +673,11 @@ SVN_ERROR_START
>>> SVN_ERR_FS_CATEGORY_START + 47,
>>> "Filesystem upgrade is not supported")
>>>
>>> + /** @since New in 1.6. */
>>> + SVN_ERRDEF(SVN_ERR_FS_NO_SUCH_CHECKSUM_REP,
>>> + SVN_ERR_FS_CATEGORY_START + 49,
>>> + "Filesystem has no such checksum-representation index
>>>
>> record")
>>
>> This should be + 48.
>>
>>
>>> ...
>>>
>>> +++ trunk/subversion/include/svn_fs.h Sat Oct 18 06:11:59 2008
>>>
>> (r33730)
>>
>>> @@ -31,6 +31,7 @@
>>> #include "svn_delta.h"
>>> #include "svn_io.h"
>>> #include "svn_mergeinfo.h"
>>> +#include "svn_checksum.h"
>>>
>> Oof. This shoulda been in there already since svn_checksum_t was in
>> use.
>>
>>
>>> ...
>>> +++ trunk/subversion/libsvn_fs_base/bdb/dbt.c Sat Oct 18 06:11:59
>>>
>> 2008 (r33730)
>>
>>> @@ -18,6 +18,8 @@
>>> #include <stdlib.h>
>>> #include <string.h>
>>> #include <apr_pools.h>
>>> +#include <apr_md5.h>
>>> +#include <apr_sha1.h>
>>>
>>> #define APU_WANT_DB
>>> #include <apu_want.h>
>>> @@ -153,3 +155,20 @@ svn_fs_base__str_to_dbt(DBT *dbt, const
>>> svn_fs_base__set_dbt(dbt, str, strlen(str));
>>> return dbt;
>>> }
>>> +
>>> +DBT *
>>> +svn_fs_base__checksum_to_dbt(DBT *dbt, svn_checksum_t *checksum)
>>> +{
>>> + switch (checksum->kind)
>>> + {
>>> + case svn_checksum_md5:
>>> + svn_fs_base__set_dbt(dbt, checksum->digest,
>>>
>> APR_MD5_DIGESTSIZE);
>>
>>> + break;
>>> +
>>> + case svn_checksum_sha1:
>>> + svn_fs_base__set_dbt(dbt, checksum->digest,
>>>
>> APR_SHA1_DIGESTSIZE);
>>
>>> + break;
>>> + }
>>> +
>>> + return dbt;
>>> +}
>>>
>> Maybe export the DIGESTSIZE() macro out of libsvn_subr/checksum.c ??
>>
>>
>>> ...
>>> +++ trunk/subversion/libsvn_fs_base/dag.c Sat Oct 18 06:11:59
>>>
>> 2008 (r33730)
>>
>>> @@ -33,6 +33,7 @@
>>> #include "reps-strings.h"
>>> #include "revs-txns.h"
>>> #include "id.h"
>>> +#include "fsguid.h"
>>>
>>> #include "util/fs_skels.h"
>>>
>>> @@ -42,6 +43,7 @@
>>> #include "bdb/copies-table.h"
>>> #include "bdb/reps-table.h"
>>> #include "bdb/strings-table.h"
>>> +#include "bdb/checksum-reps-table.h"
>>>
>>> #include "private/svn_fs_util.h"
>>> #include "../libsvn_fs/fs-loader.h"
>>> @@ -576,9 +578,15 @@ svn_fs_base__dag_set_proplist(dag_node_t
>>> trail_t *trail,
>>> apr_pool_t *pool)
>>> {
>>> + svn_error_t *err;
>>> node_revision_t *noderev;
>>> - const char *rep_key, *mutable_rep_key;
>>> + const char *rep_key, *mutable_rep_key, *dup_rep_key;
>>> svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
>>> + svn_stream_t *wstream;
>>> + apr_size_t len;
>>> + skel_t *proplist_skel;
>>> + svn_stringbuf_t *raw_proplist_buf;
>>> + svn_checksum_t *checksum;
>>>
>>> /* Sanity check: this node better be mutable! */
>>> if (! svn_fs_base__dag_check_mutable(node, txn_id))
>>> @@ -595,6 +603,34 @@ svn_fs_base__dag_set_proplist(dag_node_t
>>> trail, pool));
>>> rep_key = noderev->prop_key;
>>>
>>> + /* Flatten the proplist into a string, and calculate its md5 hash.
>>>
>> */
>>
>>> + SVN_ERR(svn_fs_base__unparse_proplist_skel(&proplist_skel,
>>> + proplist, pool));
>>> + raw_proplist_buf = svn_fs_base__unparse_skel(proplist_skel, pool);
>>> + SVN_ERR(svn_checksum(&checksum, svn_checksum_sha1,
>>>
>> raw_proplist_buf->data,
>>
>>> + raw_proplist_buf->len, pool));
>>>
>> The comment is wrong.
>>
>>
>>> +
>>> + /* If the resulting property list is exactly the same as another
>>> + string in the database, just use the previously existing string
>>> + and get outta here. */
>>> + err = svn_fs_bdb__get_checksum_rep(&dup_rep_key, fs, checksum,
>>> + trail, pool);
>>> + if (! err)
>>> + {
>>> + if (noderev->prop_key)
>>> + SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, noderev-
>>> prop_key,
>>> + txn_id, trail,
>>>
>> pool));
>>
>>> + noderev->prop_key = dup_rep_key;
>>> + return svn_fs_bdb__put_node_revision(fs, node->id, noderev,
>>> + trail, pool);
>>> + }
>>> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
>>> + {
>>> + svn_error_clear(err);
>>> + err = SVN_NO_ERROR;
>>> + }
>>> + SVN_ERR(err);
>>>
>> This last part seems a bit easier to do:
>>
>> else if (err)
>> {
>> if (err->apr_err != ...)
>> return err;
>> svn_error_clear(err);
>> }
>>
>> I mean, the SVN_ERR(err) is only useful for the else-block. The first
>> if() means you don't need to check err again. etc... it is simply more
>> checking than needed :-P
>>
>>
>>> ...
>>> + if (! err)
>>> + {
>>> + useless_data_key = noderev->edit_key;
>>> + err = svn_fs_base__reserve_fsguid(trail->fs,
>>>
>> &data_key_uniquifier,
>>
>>> + trail, pool);
>>> + }
>>> + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
>>> + {
>>> + svn_error_clear(err);
>>> + err = SVN_NO_ERROR;
>>> + new_data_key = noderev->edit_key;
>>> + }
>>> + SVN_ERR(err);
>>>
>> Same here.
>>
>>
>>> ...
>>> +++ trunk/subversion/libsvn_fs_base/fs.h Sat Oct 18 06:11:59
>>>
>> 2008 (r33730)
>>
>>> @@ -41,6 +41,9 @@ extern "C" {
>>> back-end's format. */
>>> #define SVN_FS_BASE__FORMAT_NUMBER 4
>>>
>>> +/* Minimum format number that supports representation sharing */
>>> +#define SVN_FS_BASE__MIN_REP_SHARING_FORMAT 4
>>> +
>>> /* Minimum format number that supports the 'miscellaneous' table */
>>> #define SVN_FS_BASE__MIN_MISCELLANY_FORMAT 4
>>>
>> Seems like these two MIN constants should be combined into one, for
>> simplicity.
>>
>>
>>> ...
>>> +++ trunk/subversion/libsvn_fs_base/notes/structure Sat Oct 18
>>>
>> 06:11:59 2008 (r33730)
>>
>> Maybe make FSGUID explicit?
>>
>> Also: where to note about a format upgrade that will include FSGUID?
>> If you're going to migrate all key generation to FSGUID, then you'll
>> want to start FSGUID at max(all-next-key-columns). I think if you
>> start FSGUID at "0" like the patch is currently doing, you *might*
>> close off the door to broader use. Haven't thought it through yet tho.
>> But if true, then you're going to want to do some work to get that
>> max() before we cut 1.6.
>>
>>
>>> ...
>>> +++ trunk/subversion/libsvn_fs_fs/rep-cache.c Sat Oct 18 06:11:59
>>>
>> 2008 (r33730, copy of r33729, branches/fs-rep-
>> sharing/subversion/libsvn_fs_fs/rep-cache.c)
>>
>>> ...
>>> +const char *upgrade_sql[] = { NULL,
>>> + "pragma auto_vacuum = 1;"
>>> + APR_EOL_STR
>>> + "create table rep_cache (hash text not null, "
>>> + " revision integer not null, "
>>> + " offset integer not null, "
>>> + " size integer not null, "
>>> + " expanded_size integer not null, "
>>> + " reuse_count integer not null); "
>>> + APR_EOL_STR
>>> + "create unique index i_hash on rep_cache(hash); "
>>>
>> Shouldn't need this custom index if you just mark the hash column as
>> the primary key (which would be nice for docco purposes anyways).
>>
>>
>>> ...
>>> +svn_error_t *
>>> +svn_fs_fs__get_rep_reference(representation_t **rep,
>>> + svn_fs_t *fs,
>>> + svn_checksum_t *checksum,
>>> + apr_pool_t *pool)
>>> +{
>>> + fs_fs_data_t *ffd = fs->fsap_data;
>>> + svn_boolean_t have_row;
>>> + svn_sqlite__stmt_t *stmt;
>>> +
>>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>>> + "select revision, offset, size, expanded_size from
>>>
>> rep_cache "
>>
>>> + "where hash = :1", pool));
>>>
>> I would recommend preparing this statement when you open the db. Then
>> each time this function is called, you can just bind some new values,
>> run the query, and finalize the statement. Definitely more efficient.
>>
>
> Maybe just store it the first time and reuse it after that?
>
> (I didn't check the performance impact, but it might trigger the sql
> optimizer while the query is in many cases not executed)
>
>
>>> ...
>>> +svn_error_t *
>>> +svn_fs_fs__set_rep_reference(svn_fs_t *fs,
>>> + representation_t *rep,
>>> + svn_boolean_t reject_dup,
>>> + apr_pool_t *pool)
>>> +{
>>> ...
>>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>>> + "insert into rep_cache (hash, revision, offset,
>>>
>> size, "
>>
>>> + "expanded_size, reuse_count) "
>>> + "values (:1, :2, :3, :4, :5, 0);", pool));
>>>
>> Likewise.
>>
>>
>>> ...
>>> +svn_error_t *
>>> +svn_fs_fs__inc_rep_reuse(svn_fs_t *fs,
>>> + representation_t *rep,
>>> + apr_pool_t *pool)
>>> +{
>>> + fs_fs_data_t *ffd = fs->fsap_data;
>>> + svn_boolean_t have_row;
>>> + svn_sqlite__stmt_t *stmt;
>>> +
>>> + /* Fetch the current count. */
>>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>>> + "select reuse_count from rep_cache where hash = :1",
>>>
>> pool));
>>
>>> ...
>>> + /* Update the reuse_count. */
>>> + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>>> + "update rep_cache set reuse_count = :1 where
>>>
>> hash = :2",
>>
>>> + pool));
>>>
>> And two more :-)
>>
>
> Bert
>
>>> ...
>>>
>> Cheers,
>> -g
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Bert Huijben <b....@competence.biz>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: maandag 20 oktober 2008 12:51
> To: dev@subversion.tigris.org; hwright@tigris.org
> Subject: Re: svn commit: r33730 - in trunk: . notes/tree-conflicts
> subversion/include subversion/libsvn_fs_base
> subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes
> subversion/libsvn_fs_base/util subversion/libsvn_fs_fs
> subversion/libsvn_subr subvers
>
> On Sat, Oct 18, 2008 at 6:12 AM, <hw...@tigris.org> wrote:
> >...
>
> > +++ trunk/subversion/include/svn_error_codes.h Sat Oct 18 06:11:59
> 2008 (r33730)
> > @@ -673,6 +673,11 @@ SVN_ERROR_START
> > SVN_ERR_FS_CATEGORY_START + 47,
> > "Filesystem upgrade is not supported")
> >
> > + /** @since New in 1.6. */
> > + SVN_ERRDEF(SVN_ERR_FS_NO_SUCH_CHECKSUM_REP,
> > + SVN_ERR_FS_CATEGORY_START + 49,
> > + "Filesystem has no such checksum-representation index
> record")
>
> This should be + 48.
>
> >...
>
> > +++ trunk/subversion/include/svn_fs.h Sat Oct 18 06:11:59 2008
> (r33730)
> > @@ -31,6 +31,7 @@
> > #include "svn_delta.h"
> > #include "svn_io.h"
> > #include "svn_mergeinfo.h"
> > +#include "svn_checksum.h"
>
> Oof. This shoulda been in there already since svn_checksum_t was in
> use.
>
> >...
> > +++ trunk/subversion/libsvn_fs_base/bdb/dbt.c Sat Oct 18 06:11:59
> 2008 (r33730)
> > @@ -18,6 +18,8 @@
> > #include <stdlib.h>
> > #include <string.h>
> > #include <apr_pools.h>
> > +#include <apr_md5.h>
> > +#include <apr_sha1.h>
> >
> > #define APU_WANT_DB
> > #include <apu_want.h>
> > @@ -153,3 +155,20 @@ svn_fs_base__str_to_dbt(DBT *dbt, const
> > svn_fs_base__set_dbt(dbt, str, strlen(str));
> > return dbt;
> > }
> > +
> > +DBT *
> > +svn_fs_base__checksum_to_dbt(DBT *dbt, svn_checksum_t *checksum)
> > +{
> > + switch (checksum->kind)
> > + {
> > + case svn_checksum_md5:
> > + svn_fs_base__set_dbt(dbt, checksum->digest,
> APR_MD5_DIGESTSIZE);
> > + break;
> > +
> > + case svn_checksum_sha1:
> > + svn_fs_base__set_dbt(dbt, checksum->digest,
> APR_SHA1_DIGESTSIZE);
> > + break;
> > + }
> > +
> > + return dbt;
> > +}
>
> Maybe export the DIGESTSIZE() macro out of libsvn_subr/checksum.c ??
>
> >...
> > +++ trunk/subversion/libsvn_fs_base/dag.c Sat Oct 18 06:11:59
> 2008 (r33730)
> > @@ -33,6 +33,7 @@
> > #include "reps-strings.h"
> > #include "revs-txns.h"
> > #include "id.h"
> > +#include "fsguid.h"
> >
> > #include "util/fs_skels.h"
> >
> > @@ -42,6 +43,7 @@
> > #include "bdb/copies-table.h"
> > #include "bdb/reps-table.h"
> > #include "bdb/strings-table.h"
> > +#include "bdb/checksum-reps-table.h"
> >
> > #include "private/svn_fs_util.h"
> > #include "../libsvn_fs/fs-loader.h"
> > @@ -576,9 +578,15 @@ svn_fs_base__dag_set_proplist(dag_node_t
> > trail_t *trail,
> > apr_pool_t *pool)
> > {
> > + svn_error_t *err;
> > node_revision_t *noderev;
> > - const char *rep_key, *mutable_rep_key;
> > + const char *rep_key, *mutable_rep_key, *dup_rep_key;
> > svn_fs_t *fs = svn_fs_base__dag_get_fs(node);
> > + svn_stream_t *wstream;
> > + apr_size_t len;
> > + skel_t *proplist_skel;
> > + svn_stringbuf_t *raw_proplist_buf;
> > + svn_checksum_t *checksum;
> >
> > /* Sanity check: this node better be mutable! */
> > if (! svn_fs_base__dag_check_mutable(node, txn_id))
> > @@ -595,6 +603,34 @@ svn_fs_base__dag_set_proplist(dag_node_t
> > trail, pool));
> > rep_key = noderev->prop_key;
> >
> > + /* Flatten the proplist into a string, and calculate its md5 hash.
> */
> > + SVN_ERR(svn_fs_base__unparse_proplist_skel(&proplist_skel,
> > + proplist, pool));
> > + raw_proplist_buf = svn_fs_base__unparse_skel(proplist_skel, pool);
> > + SVN_ERR(svn_checksum(&checksum, svn_checksum_sha1,
> raw_proplist_buf->data,
> > + raw_proplist_buf->len, pool));
>
> The comment is wrong.
>
> > +
> > + /* If the resulting property list is exactly the same as another
> > + string in the database, just use the previously existing string
> > + and get outta here. */
> > + err = svn_fs_bdb__get_checksum_rep(&dup_rep_key, fs, checksum,
> > + trail, pool);
> > + if (! err)
> > + {
> > + if (noderev->prop_key)
> > + SVN_ERR(svn_fs_base__delete_rep_if_mutable(fs, noderev-
> >prop_key,
> > + txn_id, trail,
> pool));
> > + noderev->prop_key = dup_rep_key;
> > + return svn_fs_bdb__put_node_revision(fs, node->id, noderev,
> > + trail, pool);
> > + }
> > + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
> > + {
> > + svn_error_clear(err);
> > + err = SVN_NO_ERROR;
> > + }
> > + SVN_ERR(err);
>
> This last part seems a bit easier to do:
>
> else if (err)
> {
> if (err->apr_err != ...)
> return err;
> svn_error_clear(err);
> }
>
> I mean, the SVN_ERR(err) is only useful for the else-block. The first
> if() means you don't need to check err again. etc... it is simply more
> checking than needed :-P
>
> >...
> > + if (! err)
> > + {
> > + useless_data_key = noderev->edit_key;
> > + err = svn_fs_base__reserve_fsguid(trail->fs,
> &data_key_uniquifier,
> > + trail, pool);
> > + }
> > + else if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_CHECKSUM_REP))
> > + {
> > + svn_error_clear(err);
> > + err = SVN_NO_ERROR;
> > + new_data_key = noderev->edit_key;
> > + }
> > + SVN_ERR(err);
>
> Same here.
>
> >...
> > +++ trunk/subversion/libsvn_fs_base/fs.h Sat Oct 18 06:11:59
> 2008 (r33730)
> > @@ -41,6 +41,9 @@ extern "C" {
> > back-end's format. */
> > #define SVN_FS_BASE__FORMAT_NUMBER 4
> >
> > +/* Minimum format number that supports representation sharing */
> > +#define SVN_FS_BASE__MIN_REP_SHARING_FORMAT 4
> > +
> > /* Minimum format number that supports the 'miscellaneous' table */
> > #define SVN_FS_BASE__MIN_MISCELLANY_FORMAT 4
>
> Seems like these two MIN constants should be combined into one, for
> simplicity.
>
> >...
> > +++ trunk/subversion/libsvn_fs_base/notes/structure Sat Oct 18
> 06:11:59 2008 (r33730)
>
> Maybe make FSGUID explicit?
>
> Also: where to note about a format upgrade that will include FSGUID?
> If you're going to migrate all key generation to FSGUID, then you'll
> want to start FSGUID at max(all-next-key-columns). I think if you
> start FSGUID at "0" like the patch is currently doing, you *might*
> close off the door to broader use. Haven't thought it through yet tho.
> But if true, then you're going to want to do some work to get that
> max() before we cut 1.6.
>
> >...
> > +++ trunk/subversion/libsvn_fs_fs/rep-cache.c Sat Oct 18 06:11:59
> 2008 (r33730, copy of r33729, branches/fs-rep-
> sharing/subversion/libsvn_fs_fs/rep-cache.c)
> >...
> > +const char *upgrade_sql[] = { NULL,
> > + "pragma auto_vacuum = 1;"
> > + APR_EOL_STR
> > + "create table rep_cache (hash text not null, "
> > + " revision integer not null, "
> > + " offset integer not null, "
> > + " size integer not null, "
> > + " expanded_size integer not null, "
> > + " reuse_count integer not null); "
> > + APR_EOL_STR
> > + "create unique index i_hash on rep_cache(hash); "
>
> Shouldn't need this custom index if you just mark the hash column as
> the primary key (which would be nice for docco purposes anyways).
>
> >...
> > +svn_error_t *
> > +svn_fs_fs__get_rep_reference(representation_t **rep,
> > + svn_fs_t *fs,
> > + svn_checksum_t *checksum,
> > + apr_pool_t *pool)
> > +{
> > + fs_fs_data_t *ffd = fs->fsap_data;
> > + svn_boolean_t have_row;
> > + svn_sqlite__stmt_t *stmt;
> > +
> > + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> > + "select revision, offset, size, expanded_size from
> rep_cache "
> > + "where hash = :1", pool));
>
> I would recommend preparing this statement when you open the db. Then
> each time this function is called, you can just bind some new values,
> run the query, and finalize the statement. Definitely more efficient.
Maybe just store it the first time and reuse it after that?
(I didn't check the performance impact, but it might trigger the sql
optimizer while the query is in many cases not executed)
>
> >...
> > +svn_error_t *
> > +svn_fs_fs__set_rep_reference(svn_fs_t *fs,
> > + representation_t *rep,
> > + svn_boolean_t reject_dup,
> > + apr_pool_t *pool)
> > +{
> >...
> > + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> > + "insert into rep_cache (hash, revision, offset,
> size, "
> > + "expanded_size, reuse_count) "
> > + "values (:1, :2, :3, :4, :5, 0);", pool));
>
> Likewise.
>
> >...
> > +svn_error_t *
> > +svn_fs_fs__inc_rep_reuse(svn_fs_t *fs,
> > + representation_t *rep,
> > + apr_pool_t *pool)
> > +{
> > + fs_fs_data_t *ffd = fs->fsap_data;
> > + svn_boolean_t have_row;
> > + svn_sqlite__stmt_t *stmt;
> > +
> > + /* Fetch the current count. */
> > + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> > + "select reuse_count from rep_cache where hash = :1",
> pool));
> >...
> > + /* Update the reuse_count. */
> > + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
> > + "update rep_cache set reuse_count = :1 where
> hash = :2",
> > + pool));
>
> And two more :-)
Bert
>
> >...
>
> Cheers,
> -g
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33730 - in trunk: . notes/tree-conflicts subversion/include subversion/libsvn_fs_base subversion/libsvn_fs_base/bdb subversion/libsvn_fs_base/notes subversion/libsvn_fs_base/util subversion/libsvn_fs_fs subversion/libsvn_subr subvers
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Oct 20, 2008 at 5:22 AM, Bert Huijben <b....@competence.biz> wrote:
>...
>> > + SVN_ERR(svn_sqlite__prepare(&stmt, ffd->rep_cache,
>> > + "select revision, offset, size, expanded_size from
>> rep_cache "
>> > + "where hash = :1", pool));
>>
>> I would recommend preparing this statement when you open the db. Then
>> each time this function is called, you can just bind some new values,
>> run the query, and finalize the statement. Definitely more efficient.
>
> Maybe just store it the first time and reuse it after that?
Yeah. That would be better.
As I started to use our internal SQLite interface, I was hoping to add
an API to do just this. "Prepare <this> statement, if I haven't
already. Okay, now bind <these> values."
It would also be neat if we had a printf-style binding API, rather
than N bind calls.
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org