Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
Kyle McKay wrote on Sat, Jul 06, 2013 at 19:46:40 -0700: On Jul 6, 2013, at 19:23, Jonathan Nieder wrote: Kyle McKay wrote: Unless bulk updates are disabled when using the serf access method (the only one available with svn 1.8) for https?: urls, apply_textdelta does indeed get called multiple times in a row without an intervening temp_release. You mean Unless bulk updates are enabled and without an intervening close_file, right? The problem seems to be skelta mode although it may just be the fact that ra_serf has multiple connections outstanding and since ra_neon only ever has one it can't happen over ra_neon. If the server disables bulk updates (SVNAllowBulkUpdates Off) all clients are forced to use skelta mode, even ra_neon clients. As Brane and I have pointed out, git-svn can instruct libsvn_* to use bulk updates regardless of the server version, by setting SVN_CONFIG_OPTION_HTTP_BULK_UPDATES (new in 1.8). If you have questions about that, though, please address them to us...@subversion.apache.org (the proper list for API usage questions), not to me personally. Cheers, Daniel -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
On 7/7/2013 6:39 AM, Daniel Shahaf wrote: Kyle McKay wrote on Sat, Jul 06, 2013 at 19:46:40 -0700: On Jul 6, 2013, at 19:23, Jonathan Nieder wrote: Kyle McKay wrote: Unless bulk updates are disabled when using the serf access method (the only one available with svn 1.8) for https?: urls, apply_textdelta does indeed get called multiple times in a row without an intervening temp_release. You mean Unless bulk updates are enabled and without an intervening close_file, right? The problem seems to be skelta mode although it may just be the fact that ra_serf has multiple connections outstanding and since ra_neon only ever has one it can't happen over ra_neon. If the server disables bulk updates (SVNAllowBulkUpdates Off) all clients are forced to use skelta mode, even ra_neon clients. As Brane and I have pointed out, git-svn can instruct libsvn_* to use bulk updates regardless of the server version, by setting SVN_CONFIG_OPTION_HTTP_BULK_UPDATES (new in 1.8). According to the table in the release notes [1], Skelta mode will be used if the 1.7 or 1.8 server sets SVNAllowBulkUpdates to Off, regardless of what the client sets in the configuration. Is that not true? [1] https://subversion.apache.org/docs/release-notes/1.8.html#neon-deleted -- David Rothenberger daver...@acm.org -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
I forwarded the SVNAllowBulkUpdates Off question to the us...@subversion.apache.org list and here's the reply: On Jul 7, 2013, at 11:11, Lieven Govaerts wrote: On Sun, Jul 7, 2013 at 4:48 PM, Kyle McKay mack...@gmail.com wrote: On Jul 7, 2013, at 06:39, Daniel Shahaf wrote: Kyle McKay wrote on Sat, Jul 06, 2013 at 19:46:40 -0700: On Jul 6, 2013, at 19:23, Jonathan Nieder wrote: Kyle McKay wrote: Unless bulk updates are disabled when using the serf access method (the only one available with svn 1.8) for https?: urls, apply_textdelta does indeed get called multiple times in a row without an intervening temp_release. You mean Unless bulk updates are enabled and without an intervening close_file, right? The problem seems to be skelta mode although it may just be the fact that ra_serf has multiple connections outstanding and since ra_neon only ever has one it can't happen over ra_neon. If the server disables bulk updates (SVNAllowBulkUpdates Off) all clients are forced to use skelta mode, even ra_neon clients. As Brane and I have pointed out, git-svn can instruct libsvn_* to use bulk updates regardless of the server version, by setting SVN_CONFIG_OPTION_HTTP_BULK_UPDATES (new in 1.8). If you have questions about that, though, please address them to us...@subversion.apache.org (the proper list for API usage questions), not to me personally. According to the table at http://subversion.apache.org/docs/release-notes/1.8.html#serf-skelta-default , if the server sets SVNAllowBulkUpdates Off, the client will be forced to use skelta no matter what the client setting is. Indeed, the server admin has the final say in which mode is actually used. SVNAllowBulkUpdates Off is only advised if the server admin wants a log line per accessed resource. I doubt it's used a lot, but the option is there. Is that table incorrect? No, that table is correct. Lieven So the final say so on whether or not bulk updates are allowed is on the server side which means git-svn really needs to handle skelta mode on the client side properly when using ra-serf to guarantee functionality with all subversion server configurations. Kyle -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
(cc-ing Eric Wong, who wrote this code)
Hi,
Kyle McKay wrote:
Temp file with moniker 'svn_delta' already in use at Git.pm line 1250
Temp file with moniker 'git_blob' already in use at Git.pm line 1250
David Rothenberger daver...@acm.org has determined the cause to
be that ra_serf does not drive the delta editor in a depth-first
manner [...]. Instead, the calls come in this order:
[...]
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -315,11 +315,13 @@ sub change_file_prop {
sub apply_textdelta {
my ($self, $fb, $exp) = @_;
return undef if $self-is_path_ignored($fb-{path});
- my $fh = $::_repository-temp_acquire('svn_delta');
+ my $suffix = 0;
+ ++$suffix while
$::_repository-temp_is_locked(svn_delta_${$}_$suffix);
+ my $fh = $::_repository-temp_acquire(svn_delta_${$}_$suffix);
# $fh gets auto-closed() by SVN::TxDelta::apply(),
# (but $base does not,) so dup() it for reading in close_file
open my $dup, '', $fh or croak $!;
- my $base = $::_repository-temp_acquire('git_blob');
+ my $base = $::_repository-temp_acquire(git_blob_${$}_$suffix);
Thanks for your work tracking this down.
I'm a bit confused. Are you saying that apply_textdelta gets called
multiple times in a row without an intervening close_file?
Puzzled,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
On Jul 6, 2013, at 17:24, Jonathan Nieder wrote:
(cc-ing Eric Wong, who wrote this code)
Hi,
Kyle McKay wrote:
Temp file with moniker 'svn_delta' already in use at Git.pm line 1250
Temp file with moniker 'git_blob' already in use at Git.pm line 1250
David Rothenberger daver...@acm.org has determined the cause to
be that ra_serf does not drive the delta editor in a depth-first
manner [...]. Instead, the calls come in this order:
[...]
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -315,11 +315,13 @@ sub change_file_prop {
sub apply_textdelta {
my ($self, $fb, $exp) = @_;
return undef if $self-is_path_ignored($fb-{path});
- my $fh = $::_repository-temp_acquire('svn_delta');
+ my $suffix = 0;
+ ++$suffix while $::_repository-temp_is_locked(svn_delta_${$}_
$suffix);
+ my $fh = $::_repository-temp_acquire(svn_delta_${$}_$suffix);
# $fh gets auto-closed() by SVN::TxDelta::apply(),
# (but $base does not,) so dup() it for reading in close_file
open my $dup, '', $fh or croak $!;
- my $base = $::_repository-temp_acquire('git_blob');
+ my $base = $::_repository-temp_acquire(git_blob_${$}_$suffix);
Thanks for your work tracking this down.
I'm a bit confused. Are you saying that apply_textdelta gets called
multiple times in a row without an intervening close_file?
Unless bulk updates are disabled when using the serf access method
(the only one available with svn 1.8) for https?: urls,
apply_textdelta does indeed get called multiple times in a row without
an intervening temp_release.
Two temp files are created for each apply_textdelta ('svn_delta...'
and 'git_blob...'). In my tests it seems that most of the time the
two temp files are enough, but occasionally as many as six will be
open at the same time.
I suspect this maximum number is related to the maximum number of
simultaneous connections the serf access method will use which
defaults to 4. Therefore I would expect to see as many as 8 temp
files (4 each for 'svn_delta...' and 'git_blob...'), but I have only
been able to trigger creation of 6 temp files so far.
Kyle
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
Kyle McKay wrote: Unless bulk updates are disabled when using the serf access method (the only one available with svn 1.8) for https?: urls, apply_textdelta does indeed get called multiple times in a row without an intervening temp_release. You mean Unless bulk updates are enabled and without an intervening close_file, right? Unlike the non-depth-first thing, that sounds basically broken --- what would be stopping subversion from calling the editor's close method when done with each file? I can't see much reason unless it is calling apply_textdelta multiple times in parallel --- is it doing that, and if so is git-svn able to cope with that? This sounds like something that should be fixed in ra_serf. But if the number of overlapping open text nodes is bounded by a low number, the workaround of using multiple temp files sounds ok as a way of dealing with unfixed versions of Subversion. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: allow git-svn fetching to work using serf
On Jul 6, 2013, at 19:23, Jonathan Nieder wrote:
Kyle McKay wrote:
Unless bulk updates are disabled when using the serf access method
(the only one available with svn 1.8) for https?: urls,
apply_textdelta does indeed get called multiple times in a row
without an intervening temp_release.
You mean Unless bulk updates are enabled and without an intervening
close_file, right?
The problem seems to be skelta mode although it may just be the fact
that ra_serf has multiple connections outstanding and since ra_neon
only ever has one it can't happen over ra_neon.
If the server disables bulk updates (SVNAllowBulkUpdates Off) all
clients are forced to use skelta mode, even ra_neon clients.
This sounds like something that should be fixed in ra_serf.
Yes, but apparently it will not be.
But if the number of overlapping open text nodes is bounded by a low
number, the workaround of using multiple temp files sounds ok as a way
of dealing with unfixed versions of Subversion.
I believe it will never exceed twice ('svn_delta...' and
'git_blob...') the maximum number of serf connections allowed. Four
by default (hard-coded prior to svn 1.8). Limited to between 1 and 8
on svn 1.8. Actually it looks like from my testing that it won't ever
exceed twice the (max number of serf connections - 1).
Kyle
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
