You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Frank Loeffler <kn...@cct.lsu.edu> on 2013/07/06 13:39:27 UTC
Parallel checkout within checkout
Hi,
I am trying to find out if some new behavior we see with version 1.7 is
considered a bug (I would).
What we try to do is assembling a larger piece of software from tens or
hundreds of repositories. In that, checkouts might also live within
checkouts of other svn repositories. We usually checkout these in
parallel, but because the contained checkout isn't using a subdirectory
name that would be part of the containing repository, this wasn't a
problem in the past.
However, version 1.7 seems to change this. Trying to checkout the
contained repository while the containing repository isn't finished yet
(parallel checkout) produces errors like
svn: E155037: Previous operation has not finished; run 'cleanup' if it
was interrupted
My theory is that the checkout of the contained repository tries to look
into .svn of the containing repository, and finds it 'currently used'.
This was not the case for versions <1.7, maybe because these didn't use
a 'central' .svn directory.
I would now argue that this is a regression, since svn <1.7 worked fine
in this scenario, while >=1.7 doesn't.
Ways to reproduce:
1. svn co some_repo
2. interrupt during checkout (control-Z)
3. cd into new (partial) checkout
4. svn co some_other_repo
thanks, Frank Loeffler
Re: Parallel checkout within checkout
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jul 09, 2013 at 10:03:30AM +0200, Stefan Sperling wrote:
> On Mon, Jul 08, 2013 at 10:51:54PM -0500, Frank Loeffler wrote:
> > Would this now be a good time to open a ticket to try to get this in a
> > future release version?
>
> No need, I'll try to take care of this fix myself.
> Thanks for your report!
There are some open questions about this fix. I've filed
http://subversion.tigris.org/issues/show_bug.cgi?id=4390
Please add yourself to the Cc list there if you want to stay
informed about progress on this issue.
Re: Parallel checkout within checkout
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jul 08, 2013 at 10:51:54PM -0500, Frank Loeffler wrote:
> Would this now be a good time to open a ticket to try to get this in a
> future release version?
No need, I'll try to take care of this fix myself.
Thanks for your report!
Re: Parallel checkout within checkout
Posted by Frank Loeffler <kn...@cct.lsu.edu>.
Hi Stefan
On Mon, Jul 08, 2013 at 11:18:15PM +0200, Stefan Sperling wrote:
> This patch is sufficient to make it work in my testing.
> Can you confirm?
I can. Thanks a lot!
I downloaded the source of 1.8.0 and compiled that (Debian Wheezy plus a
new copy of serf because I wanted to test using http repositories and
the version of serf in Wheeze is too old). I verified that the problem
still persists with that version using the procedure I mentioned earlier
(ctrl+Z, ...). I then applied the patch and it now works. I tried
several times to be sure and succeeded every time.
Would this now be a good time to open a ticket to try to get this in a
future release version?
Frank
Re: Parallel checkout within checkout
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jul 07, 2013 at 01:39:33PM -0500, Frank Loeffler wrote:
> On Sun, Jul 07, 2013 at 06:34:09PM +0200, Stefan Sperling wrote:
> > By design, there are many cases where the working copy code ends up
> > searching the directory hierarchy upwards for a wc.db database in
> > a .svn directory.
>
> Does it stop once it finds one, or does it continue to search further
> up? I would assume it stops, since it only needs one database. If it
> stops, then a new checkout would only be 'in trouble', as long as it's
> own database isn't present yet if I understand this correctly. Then the
> next question would be when this is created during a checkout: close to
> the beginning or when the checkout is finished. It probably has to be
> closer to the beginning, since the checkout code has to work with that
> new database, and not another that might happen to live in some parent
> directory. If this is the case, then I would imagine that the critical
> code during checkout that shouldn't look 'up' might not be that large.
>
> Of course, this chain of arguments contained a lot of assumptions and it
> would take someone more familiar with the code to comment on this.
> Depending on the results of such an initial assessment someone might be
> willing to look into it, or maybe not.
>
> Frank
This patch is sufficient to make it work in my testing.
Can you confirm?
Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 1500871)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -1863,6 +1863,35 @@ svn_wc__acquire_write_lock_for_resolve(const char
const char *local_abspath,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+
+/** Create a new administrative area for @a local_abspath, so
+ * that @a local_abspath is a working copy subdir based on @a url at @a
+ * revision, with depth @a depth, and with repository UUID @a repos_uuid
+ * and repository root URL @a repos_root_url.
+ *
+ * @a depth must be a definite depth, it cannot be #svn_depth_unknown.
+ * @a repos_uuid and @a repos_root_url MUST NOT be @c NULL, and
+ * @a repos_root_url must be a prefix of @a url.
+ *
+ * If the administrative area already exists, raise an error.
+ *
+ * Do not ensure existence of @a local_abspath itself; if @a local_abspath
+ * does not exist, return error.
+ *
+ * Use @a scratch_pool for temporary allocations.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_wc__init_adm(svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *url,
+ const char *repos_root_url,
+ const char *repos_uuid,
+ svn_revnum_t revision,
+ svn_depth_t depth,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/libsvn_client/checkout.c
===================================================================
--- subversion/libsvn_client/checkout.c (revision 1500871)
+++ subversion/libsvn_client/checkout.c (working copy)
@@ -58,9 +58,9 @@ initialize_area(const char *local_abspath,
depth = svn_depth_infinity;
/* Make the unversioned directory into a versioned one. */
- SVN_ERR(svn_wc_ensure_adm4(ctx->wc_ctx, local_abspath, pathrev->url,
- pathrev->repos_root_url, pathrev->repos_uuid,
- pathrev->rev, depth, pool));
+ SVN_ERR(svn_wc__init_adm(ctx->wc_ctx, local_abspath, pathrev->url,
+ pathrev->repos_root_url, pathrev->repos_uuid,
+ pathrev->rev, depth, pool));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c (revision 1500871)
+++ subversion/libsvn_wc/adm_files.c (working copy)
@@ -517,6 +517,32 @@ svn_wc_ensure_adm4(svn_wc_context_t *wc_ctx,
}
svn_error_t *
+svn_wc__init_adm(svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *url,
+ const char *repos_root_url,
+ const char *repos_uuid,
+ svn_revnum_t revision,
+ svn_depth_t depth,
+ apr_pool_t *scratch_pool)
+
+{
+ const char *repos_relpath = svn_uri_skip_ancestor(repos_root_url, url,
+ scratch_pool);
+
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ SVN_ERR_ASSERT(url != NULL);
+ SVN_ERR_ASSERT(repos_root_url != NULL);
+ SVN_ERR_ASSERT(repos_uuid != NULL);
+ SVN_ERR_ASSERT(repos_relpath != NULL);
+ SVN_ERR_ASSERT(depth != svn_depth_unknown);
+
+ return svn_error_trace(init_adm(wc_ctx->db, local_abspath,
+ repos_relpath, repos_root_url, repos_uuid,
+ revision, depth, scratch_pool));
+}
+
+svn_error_t *
svn_wc__adm_destroy(svn_wc__db_t *db,
const char *dir_abspath,
svn_cancel_func_t cancel_func,
Re: Parallel checkout within checkout
Posted by Frank Loeffler <kn...@cct.lsu.edu>.
On Sun, Jul 07, 2013 at 06:34:09PM +0200, Stefan Sperling wrote:
> By design, there are many cases where the working copy code ends up
> searching the directory hierarchy upwards for a wc.db database in
> a .svn directory.
Does it stop once it finds one, or does it continue to search further
up? I would assume it stops, since it only needs one database. If it
stops, then a new checkout would only be 'in trouble', as long as it's
own database isn't present yet if I understand this correctly. Then the
next question would be when this is created during a checkout: close to
the beginning or when the checkout is finished. It probably has to be
closer to the beginning, since the checkout code has to work with that
new database, and not another that might happen to live in some parent
directory. If this is the case, then I would imagine that the critical
code during checkout that shouldn't look 'up' might not be that large.
Of course, this chain of arguments contained a lot of assumptions and it
would take someone more familiar with the code to comment on this.
Depending on the results of such an initial assessment someone might be
willing to look into it, or maybe not.
Frank
Re: Parallel checkout within checkout
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jul 07, 2013 at 10:56:03AM -0500, Frank Loeffler wrote:
> As to the original problem: is there any chance the 'nested' checkout
> could be made to work? Subversion could just ignore any upper-level .svn
> directories. What is done here is a fresh checkout. There is no need to
> check anything further up in the directory tree, especially not if it
> could make that checkout fail.
By design, there are many cases where the working copy code ends up
searching the directory hierarchy upwards for a wc.db database in
a .svn directory. Basically, any time the working copy management code
is entered, the database is looked up from a cache or searched on disk.
That code has many entry points internally, so there may be many cases
where checkout ends up scanning upwards. It could also be a small number
of cases. I'm not quite sure.
We'd have to figure out where checkout hits these problems, write test
cases for them, and special-case some checkout code paths to avoid
scanning upwards. This is tedious work. But if you found a volunteer
to do it I don't think we would reject such changes principle.
The only command that explicitly avoids scanning upwards right now
is the 'svn upgrade' command, to avoid upgrading enclosing working
copies. Internally, it has to tip-toe around a lot of corners for this :(
Re: Parallel checkout within checkout
Posted by Frank Loeffler <kn...@cct.lsu.edu>.
Hi,
On Sun, Jul 07, 2013 at 05:44:24PM +0200, Stefan Sperling wrote:
> You mean "working copy" (the copy being checked out), not "repository"
> (the database we check out from).
I do, I am sorry for my lax words.
> I would argue that you were relying on an implementation detail.
> Nowhere does the documentation mention or even recommend what you
> are doing.
I don't think the documentation talks about what happens when using
'svn' in parallel at all, that is right. What exactly is called a
regression depends on how you define it, but I am not really interested
in an argument about that. It did work before and doesn't right now.
> I have two suggestions:
>
> When using 1.7 clients, run checkouts in parallel but into temporary
> directories that are not nested. When done, move the temporary working
> copies into each other to create the nested structure. This should work.
> If you keep all temp working copies on the same disk then moving them
> should be very cheap.
That is most likely the only general way we could go with this. It means
additional bookkeeping, unfortunately though.
> If you use HTTP to access the repository, my other suggestion is to update
> both clients and server to 1.8 and test checkout performance. You might
> benefit from the new HTTP client layer (serf) in skelta mode, which might
> improve throughput to the point where you don't need to checkout different
> working copies in parallel for performance reasons. See here for details:
> http://subversion.apache.org/docs/release-notes/1.8.html#neon-deleted
We use parallel checkouts most of all not because a special server might
be slow, but connections to servers might be (different continent, ect),
and because we checkout from a couple of different servers at the same
time. But we will sure look into the new version as well.
Thank you very much for your suggestions.
As to the original problem: is there any chance the 'nested' checkout
could be made to work? Subversion could just ignore any upper-level .svn
directories. What is done here is a fresh checkout. There is no need to
check anything further up in the directory tree, especially not if it
could make that checkout fail.
Frank
Re: Parallel checkout within checkout
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Jul 06, 2013 at 06:39:27AM -0500, Frank Loeffler wrote:
> My theory is that the checkout of the contained repository tries to look
> into .svn of the containing repository, and finds it 'currently used'.
You mean "working copy" (the copy being checked out), not "repository"
(the database we check out from).
Yes, I believe what you are seeing is that the nested 'svn checkout'
operation ends up using the .svn directory of the enclosing working
copy.
> This was not the case for versions <1.7, maybe because these didn't use
> a 'central' .svn directory.
>
> I would now argue that this is a regression, since svn <1.7 worked fine
> in this scenario, while >=1.7 doesn't.
I would argue that you were relying on an implementation detail.
Nowhere does the documentation mention or even recommend what you
are doing.
> Ways to reproduce:
>
> 1. svn co some_repo
> 2. interrupt during checkout (control-Z)
> 3. cd into new (partial) checkout
> 4. svn co some_other_repo
I have two suggestions:
When using 1.7 clients, run checkouts in parallel but into temporary
directories that are not nested. When done, move the temporary working
copies into each other to create the nested structure. This should work.
If you keep all temp working copies on the same disk then moving them
should be very cheap.
If you use HTTP to access the repository, my other suggestion is to update
both clients and server to 1.8 and test checkout performance. You might
benefit from the new HTTP client layer (serf) in skelta mode, which might
improve throughput to the point where you don't need to checkout different
working copies in parallel for performance reasons. See here for details:
http://subversion.apache.org/docs/release-notes/1.8.html#neon-deleted