subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <br...@wandisco.com>
Subject Re: svn commit: r1676535 -/subversion/trunk/subversion/tests/libsvn_subr/io-test.c
Date Tue, 28 Apr 2015 17:15:41 GMT
On 28.04.2015 18:54, Bert Huijben wrote:
> Not on a pc, but this uses a Windows function that allows moving the
> file while it is still open. Normally when the file is reopened for
> moving by the os, virus scanners will lock the file until they are
> sure they are safe. In many cases involving our retry logic)
>
> This moves this check to *after we are done* instead of between the
> flush and the rename. So the virus scanner still does its work but
> without delaying file installs.
>
> The performance improvement is quite visible with virus scanners and
> on network shares where this avoids quite a but I client-server
> synchronization. (Even more when combined)
>
> Bert
>
> ------------------------------------------------------------------------
> From: Ivan Zhakov <mailto:ivan@visualsvn.com>
> Sent: ‎28-‎4-‎2015 17:48
> To: Philip Martin <mailto:philip.martin@wandisco.com>
> Cc: dev@subversion.apache.org <mailto:dev@subversion.apache.org>;
> brane@apache.org <mailto:brane@apache.org>
> Subject: Re: svn commit: r1676535
> -/subversion/trunk/subversion/tests/libsvn_subr/io-test.c
>
> On 28 April 2015 at 18:42, Philip Martin <philip.martin@wandisco.com>
> wrote:
> > Ivan Zhakov <ivan@visualsvn.com> writes:
> >
> >>> What does "can be installed using Windows checkouts much slower than
> >>> Unix" mean?
> >>>
> >> I have no idea. This function was added in r1559758 [1] as preparation
> >> to fix issue #4450 "Windows checkouts much slower then Unix" [2]. May
> >> issue summary leaked to function docstring for some reason.
> >
> > I suspect it should be "can be installed using platform specific
> > optimizations".  Perhaps with something additional about closing the
> > stream before installing.
> >
> > Is closing before installing just allowed or is it required?  Before
> > your commit I wrote a patch that closed the stream *after* calling
> > svn_stream__install_stream and that was sufficient for the test to pass.
> >
> As far I understand from learning code close is *required* and should
> be performed *before* calling svn_stream__install_stream(). Calling
> svn_stream_close() after svn_stream__install_stream() works because OS
> just flushes buffers to new location, but it's platform specific
> behavior and data may be lost if power off between move and flush for
> some reason.


I think the reason for "requiring" the close before install is that we
don't have an svn_stream_flush function. In the case of install streams,
the close handler is hacked to flush the file on Windows, and the stream
knows the underlying file name, so it the seemingly illogical sequence
of "close then use" happens to work. It's just not intuitive.

-- Brane

Mime
View raw message