On Thu, May 28, 2015 at 10:55 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
> In this case the log message documented that it fixed ‘a problem on Windows’, which would suggest at least some Windows testing
No, it does not. The problems were sublte but could be
deduced from reading the source code alone (I wondered
why the code did not error out on Windows, so I started
reading through the called functions). In fact, without the
flush, the Win32 code (just call rename) is equivalent to
what was implemented previously.
The kind of testing Ivan has been asking for is to actually
demonstrate a data loss due to power failure. While I do
have access to a Windows VM for ordinary testing in our
data center, I don't have one that I can simply pull the
> Correctly fsync() after renames in FSFS on Win32. We must flush the disk
> buffers after the rename, otherwise the metadata may not be persistent.
> Moreover, if the rename is degraded to a copy by Win32, we won't even have
> the complete file contents on disk without a buffer flush.
> The original code worked just fine in 1.7 and 1.8 before this patch, so some explanation on what problem is fixed, how to reproduce the problem and how this impacts performance would be welcome.
The two problems are mentioned: The first one is that
rename itself is obviously not persistet, e.g. the 'current'
file may show the old contents even after the server
send a "commit done, new rev available" to the client.
To reproduce: Have a "busy" server setup, do lots of
commits, record HEAD revs returned to the client on
the client, pull the plug on the server and compare
HEAD on server vs. HEAD on client. In some cases,
the latter should be higher.
Secondly, if the rename degrades to a copy, the file
contents (e.g. revision data) may be incomplete upon
power failure. This one is arguably the more severe
problem as it may leave the repo in a corrupted state.
To reproduce: Have a "busy" server setup, put the repo's
protorev folder on a separate volume, do lots of commits,
pull the plug on the server and verify the repos. In some
cases, the revision data should be corrupt, e.g. just be
Ideally, you place the repo on some SAN and keep that
running such that HW buffer effects don't play a role here.
> On 28 May 2015 6:54 pm, "Ivan Zhakov" <ivan_at_visualsvn.com> wrote:
> > >> Did
> > >> you able to reproduce data corruption on VM?
> > >
> > >
> > > No. But I did not even try.
> > >
> > I really hope that you have tested your patch on Windows before commit.
> I really hope you're joking. We've never required or expected testing on all platforms, from anyone.
> -- Brane
Received on 2015-05-29 08:50:23 CEST