threads / patch / 43365

patchRe: [PATCH] git-svn: error out when the SVN connection fails during a fetch

Subject: Re: [PATCH] git-svn: error out when the SVN connection fails during a fetch

## tl;dr

9 messages between Nov 28, 2006 and Dec 6, 2006. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Eric Wong· Nov 28, 2006, 22:06 UTC · lore

[PATCH] git-svn: error out when the SVN connection fails during a fetch

finish_report does seem to return a useful value indicating success or failure, so we'll just set a flag when close_edit is called (it is not called on failures, nor is abort_edit) and check the flag before proceeding.

Thanks to Pazu for pointing this out.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Show changes to git-svn.perl +8 −1
diff --git a/git-svn.perl b/git-svn.perl
index 7942bba..c3ad5ec 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2941,6 +2941,9 @@ sub libsvn_fetch_delta {
 	$reporter->set_path('', $last_rev, 0, @lock, $pool);
 	$reporter->finish_report($pool);
 	$pool->clear;
+	unless ($ed->{git_commit_ok}) {
+		die "SVN connection failed somewhere...\n";
+	}
 	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit]);
 }
 
@@ -3195,6 +3198,9 @@ sub libsvn_new_tree {
 		$reporter->set_path('', $rev, 1, @lock, $pool);
 		$reporter->finish_report($pool);
 		$pool->clear;
+		unless ($ed->{git_commit_ok}) {
+			die "SVN connection failed somewhere...\n";
+		}
 	} else {
 		open my $gui, '| git-update-index -z --index-info' or croak $!;
 		libsvn_traverse($gui, '', $SVN->{svn_path}, $rev);
@@ -3501,7 +3507,8 @@ sub abort_edit {
 
 sub close_edit {
 	my $self = shift;
-	close $self->{gui} or croak;
+	close $self->{gui} or croak $!;
+	$self->{git_commit_ok} = 1;
 	$self->SUPER::close_edit(@_);
 }
 

-- 
1.4.4.1.ge151
Eric Wong· Nov 28, 2006, 22:24 UTC · re: Eric Wong · lore
Eric Wong <normalperson@yhbt.net> wrote:
> finish_report does seem to return a useful value indicating success
> or failure, so we'll just set a flag when close_edit is called
> (it is not called on failures, nor is abort_edit) and check
> the flag before proceeding.

Also, this only affects the delta fetcher. When using the non-delta fetch: the SVN libraries will just segfault :)

Florian Weimer· Dec 3, 2006, 18:23 UTC · re: Eric Wong · lore
* Eric Wong:
> finish_report does seem to return a useful value indicating success
> or failure, so we'll just set a flag when close_edit is called
> (it is not called on failures, nor is abort_edit) and check
> the flag before proceeding.

It seems that this needs some kind of fine-tuning. Now that git-svn uses HTTP keepalive connections, you get a HTTP request error once you run into the server-side request limit. It seems a bit excessive to

Eric Wong· Dec 4, 2006, 08:52 UTC · re: Florian Weimer · lore
Florian Weimer <fw@deneb.enyo.de> wrote:
Show 11 quoted lines
> * Eric Wong:
> 
> > finish_report does seem to return a useful value indicating success
> > or failure, so we'll just set a flag when close_edit is called
> > (it is not called on failures, nor is abort_edit) and check
> > the flag before proceeding.
> 
> It seems that this needs some kind of fine-tuning.  Now that git-svn
> uses HTTP keepalive connections, you get a HTTP request error once you
> run into the server-side request limit.  It seems a bit excessive to
> stop completely in this case.
Does the following patch help?
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 4 Dec 2006 00:51:16 -0800
Subject: [PATCH] git-svn: avoid network timeouts for long-running fetches

Long-running fetches run inside children to avoid memory leaks. When we refork, the connection in the parent can be idle for a long time; attempting to reuse it in the next child can result in timeouts.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to git-svn.perl +1 −0
diff --git a/git-svn.perl b/git-svn.perl
index d0bd0bd..747daf0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -459,6 +459,7 @@ sub fetch_lib {
 		$min = $max + 1;
 		$max += $inc;
 		$max = $head if ($max > $head);
+		$SVN = libsvn_connect($SVN_URL);
 	}
 	restore_index($index);
 	return { revision => $last_rev, commit => $last_commit };
Eric Wong· Dec 4, 2006, 09:05 UTC · re: Florian Weimer · lore
Florian Weimer <fw@deneb.enyo.de> wrote:
Show 7 quoted lines
> * Eric Wong:
> 
> > Does the following patch help?
> 
> Don't think so.  The issue is not timing-related.  I've seen a failure
> every 1000 requests, which suggests to me that it's hitting the
> MaxKeepAliveRequests limit configured on the server.

Actually, that's exactly what this patch should fix. git-svn restarts a child every 1000 revisions to avoid memory usage from going through the roof.

Florian Weimer· Dec 6, 2006, 17:01 UTC · re: Eric Wong · lore
* Eric Wong:
Show 12 quoted lines
> Florian Weimer <fw@deneb.enyo.de> wrote:
>> * Eric Wong:
>> 
>> > Does the following patch help?
>> 
>> Don't think so.  The issue is not timing-related.  I've seen a failure
>> every 1000 requests, which suggests to me that it's hitting the
>> MaxKeepAliveRequests limit configured on the server.
>
> Actually, that's exactly what this patch should fix.  git-svn restarts a
> child every 1000 revisions to avoid memory usage from going through the
> roof.
Junio C Hamano· Dec 6, 2006, 19:01 UTC · re: Florian Weimer · lore
Florian Weimer <fw@deneb.enyo.de> writes:
Show 16 quoted lines
> * Eric Wong:
>
>> Florian Weimer <fw@deneb.enyo.de> wrote:
>>> * Eric Wong:
>>> 
>>> > Does the following patch help?
>>> 
>>> Don't think so.  The issue is not timing-related.  I've seen a failure
>>> every 1000 requests, which suggests to me that it's hitting the
>>> MaxKeepAliveRequests limit configured on the server.
>>
>> Actually, that's exactly what this patch should fix.  git-svn restarts a
>> child every 1000 revisions to avoid memory usage from going through the
>> roof.
>
> Oh.  My (limited) testing of the patch confirms that.
Ok, so this means I should apply this one, right?
-- >8 --
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 4 Dec 2006 00:51:16 -0800
Subject: [PATCH] git-svn: avoid network timeouts for long-running fetches

Long-running fetches run inside children to avoid memory leaks. When we refork, the connection in the parent can be idle for a long time; attempting to reuse it in the next child can result in timeouts.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to git-svn.perl +1 −0
diff --git a/git-svn.perl b/git-svn.perl
index d0bd0bd..747daf0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -459,6 +459,7 @@ sub fetch_lib {
 		$min = $max + 1;
 		$max += $inc;
 		$max = $head if ($max > $head);
+		$SVN = libsvn_connect($SVN_URL);
 	}
 	restore_index($index);
 	return { revision => $last_rev, commit => $last_commit };

-- 
1.4.4.1.g6129
Eric Wong· Dec 6, 2006, 19:45 UTC · re: Junio C Hamano · lore
Junio C Hamano <junkio@cox.net> wrote:
Show 20 quoted lines
> Florian Weimer <fw@deneb.enyo.de> writes:
> 
> > * Eric Wong:
> >
> >> Florian Weimer <fw@deneb.enyo.de> wrote:
> >>> * Eric Wong:
> >>> 
> >>> > Does the following patch help?
> >>> 
> >>> Don't think so.  The issue is not timing-related.  I've seen a failure
> >>> every 1000 requests, which suggests to me that it's hitting the
> >>> MaxKeepAliveRequests limit configured on the server.
> >>
> >> Actually, that's exactly what this patch should fix.  git-svn restarts a
> >> child every 1000 revisions to avoid memory usage from going through the
> >> roof.
> >
> > Oh.  My (limited) testing of the patch confirms that.
> 
> Ok, so this means I should apply this one, right?
Yes, please do.  Thanks.

← back to recent threads