On Thu, May 28, 2015 at 5:54 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 28 May 2015 at 18:45, <stefan2_at_apache.org> wrote:
> > Author: stefan2
> > Date: Thu May 28 15:45:55 2015
> > New Revision: 1682265
> >
> > URL: http://svn.apache.org/r1682265
> > Log:
> > 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.
> >
> Are you sure about this change?
Yes. APR calls MoveFileEx with MOVEFILE_COPY_ALLOWED
but not with MOVEFILE_WRITE_THROUGH. If your txn folder
points to a different volume than the repo, *no* data will be fsync'ed.
> From my analysis that I posted two
> years ago [1] metadata changes are persistent fine on Windows, the
> only thing we need to make sure to flush data changes before move.
I don't see analysis of your's concerning metadata in that thread.
It is well possible that metadata changes are nicely serialized /
atomic in NTFS, so it may be hard to produce inconsistent
data within a given volume.
However, it does not tell you anything about consistency with
outside parties, say some svnsync'ed repository. The problem
is that Windows may end up not persisting the rename (of e.g.
the 'current' file) after telling the client that the commit got through
and a new revision number is available.
So, in your analysis, you would have to compare the process
output / state ("I completed data for iteration X") with the on-disk
contents after the power-off.
> Did
> you able to reproduce data corruption on VM?
>
No. But I did not even try.
> This change should significantly degrade performance of commit
> operation, so it should be done only if we are really sure that this
> code required.
>
We need to change to change the way we use fsync anyway.
I have a patch set for FSX sitting around for weeks now that
reduces the overall number of fsyncs per commit from 8 to 2.
I'll explain it in a different post.
-- Stefan^2.
Received on 2015-05-28 18:25:32 CEST