You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Michael Price <mi...@acm.org> on 2003/04/15 05:21:00 UTC
ra_svn failure over ssh on release branch.
Merge test 5 is failing for me over ra_svn via an ssh tunnel. The
relevant portion of tests.log is attached. Using release branch
http://svn.collab.net/repos/svn/branches/release-0.21
Michael
Re: ra_svn failure over ssh on release branch.
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2003-04-15 at 13:31, Philip Martin wrote:
> Now when we get the tmpfile1 contents (step 4) they are not
> immediately flushed to disk, that doesn't happen until the close.
> Before that happens we open the second RA session (step 5), which
> calls apr_proc_create to start the ssh process. This involves a
> fork/exec and the child process appears to flush it's copy of
> tmpfile1, probably in a pool cleanup handler. Then the parent process
> closes tmpfile1 (step 7) flushing the parents version of the file.
> The result is that tmpfile1 gets the buffered data twice.
Oh, right, of course.
I have reported this as a bug in APR in the past (child cleanup handlers
for files should not flush buffers), but it hasn't generated much
interest there. Your patch looks good as a workaround on the svn side.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Philip Martin <ph...@codematters.co.uk>.
Greg Hudson <gh...@MIT.EDU> writes:
> On Tue, 2003-04-15 at 10:10, Philip Martin wrote:
> > The following patch also appears to do the trick. I think we have a
> > stream, or buffering, or flushing problem somewhere.
>
> In my experience, the problem only happens sometimes; I always blamed it
> on a concurrency problem in libsvn_fs, in which case your patch may
> adjust the timing a little bit (thus masking the problem on your
> system), or it may do nothing, and you were just lucky with the tests
> you did after applying the patch.
This is what happens
1) open tmpfile1
2) open tmpfile2
3) open RA session1
4) get tmpfile1 contents
5) open RA session
6) get tmpfile2 contents
7) close tmpfile1
8) close tmpfile2
Now when we get the tmpfile1 contents (step 4) they are not
immediately flushed to disk, that doesn't happen until the close.
Before that happens we open the second RA session (step 5), which
calls apr_proc_create to start the ssh process. This involves a
fork/exec and the child process appears to flush it's copy of
tmpfile1, probably in a pool cleanup handler. Then the parent process
closes tmpfile1 (step 7) flushing the parents version of the file.
The result is that tmpfile1 gets the buffered data twice.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2003-04-15 at 15:27, Philip Martin wrote:
> Is it an important bug? It's always been been there and I don't
> recall a report before yours. It doesn't affect ordinary (i.e. not
> single file) merge. If it triggers it is most likely to cause an
> erroneous conflict, not data loss. I think it is acceptable to
> release with this bug, or to patch it.
I agree that this is an acceptable bug for the release, since it was
present in previous releases.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Philip Martin <ph...@codematters.co.uk>.
Michael Price <mi...@lmco.com> writes:
> Well, the problem is happening on my FreeBSD box at home so I have to
> wait until later tonight when I get home to test out the patch Philip
> sent.
I committed a trunk fix in r5631.
> All other tests over ra_local, ra_svn, and ra_dav are
> passing. I'll email results later. If the patch doesn't work then is
> this an issue that would hold up release or would we just release a
> patch for it later?
Is it an important bug? It's always been been there and I don't
recall a report before yours. It doesn't affect ordinary (i.e. not
single file) merge. If it triggers it is most likely to cause an
erroneous conflict, not data loss. I think it is acceptable to
release with this bug, or to patch it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Michael Price <mi...@lmco.com>.
Greg Hudson wrote:
> On Tue, 2003-04-15 at 10:10, Philip Martin wrote:
>
>>The following patch also appears to do the trick. I think we have a
>>stream, or buffering, or flushing problem somewhere.
>
> In my experience, the problem only happens sometimes; I always blamed it
> on a concurrency problem in libsvn_fs, in which case your patch may
> adjust the timing a little bit (thus masking the problem on your
> system), or it may do nothing, and you were just lucky with the tests
> you did after applying the patch.
>
> You could be right, though. Actual debugging effort would be best here.
> :)
Well, the problem is happening on my FreeBSD box at home so I have to
wait until later tonight when I get home to test out the patch Philip
sent. All other tests over ra_local, ra_svn, and ra_dav are passing.
I'll email results later. If the patch doesn't work then is this an
issue that would hold up release or would we just release a patch for it
later?
--
Michael H. Price II Member of the Engineering Staff
Lockheed Martin -- Advanced Technology Laboratories
3 Executive Campus, 6th Floor, Cherry Hill, NJ 08002
856-792-9746, fax 856-792-9925, email mprice@atl.lmco.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2003-04-15 at 10:10, Philip Martin wrote:
> The following patch also appears to do the trick. I think we have a
> stream, or buffering, or flushing problem somewhere.
In my experience, the problem only happens sometimes; I always blamed it
on a concurrency problem in libsvn_fs, in which case your patch may
adjust the timing a little bit (thus masking the problem on your
system), or it may do nothing, and you were just lucky with the tests
you did after applying the patch.
You could be right, though. Actual debugging effort would be best here.
:)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> Michael Price <mi...@acm.org> writes:
>
> > Merge test 5 is failing for me over ra_svn via an ssh tunnel. The
> > relevant portion of tests.log is attached. Using release branch
> > http://svn.collab.net/repos/svn/branches/release-0.21
>
> Try the following patch. No, I don't know why it appears to make a
> difference--there is probably some underlying bug.
>
> * subversion/libsvn_client/diff.c (do_single_file_merge): Only use one
> RA session.
That patch would break the ability to merge the difference between two
totally different repositories (I never liked that idea myself...).
The following patch also appears to do the trick. I think we have a
stream, or buffering, or flushing problem somewhere.
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 5622)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1042,9 +1042,12 @@
SVN_ERR (ra_lib->get_file (session1, "", rev1, fstream1,
NULL, &props1, pool));
+ status = apr_file_close (fp1);
+ if (status)
+ return svn_error_createf (status, NULL, "failed to close '%s'.",
+ tmpfile1);
/* ### heh, funny. we could be fetching two fulltexts from two
*totally* different repositories here. :-) */
-
SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, merge_b->url, pool));
SVN_ERR (svn_client__open_ra_session (&session2, ra_lib, merge_b->url,
auth_dir, NULL, NULL, FALSE,
@@ -1054,10 +1057,6 @@
SVN_ERR (ra_lib->get_file (session2, "", rev2, fstream2, NULL,
&props2, pool));
- status = apr_file_close (fp1);
- if (status)
- return svn_error_createf (status, NULL, "failed to close '%s'.",
- tmpfile1);
status = apr_file_close (fp2);
if (status)
return svn_error_createf (status, NULL, "failed to close '%s'.",
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn failure over ssh on release branch.
Posted by Philip Martin <ph...@codematters.co.uk>.
Michael Price <mi...@acm.org> writes:
> Merge test 5 is failing for me over ra_svn via an ssh tunnel. The
> relevant portion of tests.log is attached. Using release branch
> http://svn.collab.net/repos/svn/branches/release-0.21
Try the following patch. No, I don't know why it appears to make a
difference--there is probably some underlying bug.
* subversion/libsvn_client/diff.c (do_single_file_merge): Only use one
RA session.
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 5622)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1008,7 +1008,7 @@
const char *mimetype1, *mimetype2;
svn_string_t *pval;
apr_array_header_t *propchanges;
- void *ra_baton, *session1, *session2;
+ void *ra_baton, *session;
svn_ra_plugin_t *ra_lib;
svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
svn_wc_notify_state_t text_state = svn_wc_notify_state_unknown;
@@ -1034,24 +1034,19 @@
SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, URL1, pool));
- SVN_ERR (svn_client__open_ra_session (&session1, ra_lib, URL1, auth_dir,
+ SVN_ERR (svn_client__open_ra_session (&session, ra_lib, URL1, auth_dir,
NULL, NULL, FALSE, TRUE,
merge_b->ctx, pool));
SVN_ERR (svn_client__get_revision_number
- (&rev1, ra_lib, session1, revision1, NULL, pool));
- SVN_ERR (ra_lib->get_file (session1, "", rev1, fstream1,
+ (&rev1, ra_lib, session, revision1, NULL, pool));
+ SVN_ERR (ra_lib->get_file (session, "", rev1, fstream1,
NULL, &props1, pool));
/* ### heh, funny. we could be fetching two fulltexts from two
*totally* different repositories here. :-) */
-
- SVN_ERR (svn_ra_get_ra_library (&ra_lib, ra_baton, merge_b->url, pool));
- SVN_ERR (svn_client__open_ra_session (&session2, ra_lib, merge_b->url,
- auth_dir, NULL, NULL, FALSE,
- TRUE, merge_b->ctx, pool));
SVN_ERR (svn_client__get_revision_number
- (&rev2, ra_lib, session2, merge_b->revision, NULL, pool));
- SVN_ERR (ra_lib->get_file (session2, "", rev2, fstream2, NULL,
+ (&rev2, ra_lib, session, merge_b->revision, NULL, pool));
+ SVN_ERR (ra_lib->get_file (session, "", rev2, fstream2, NULL,
&props2, pool));
status = apr_file_close (fp1);
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org