subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
Subject Re: svn commit: r1694502 [1/6] - in /subversion/trunk: ./ notes/subversion/include/private/ subversion/libsvn_fs_x/ subversion/libsvn_subr/subversion/tests/cmdline/ subversion/tests/libsvn_fs/subversion/tests/libsvn_fs_x/
Date Fri, 07 Aug 2015 13:02:18 GMT
On Fri, Aug 7, 2015 at 1:25 AM, <bert@qqmail.nl> wrote:

> [And now to the proper list]
>
>
>
> On the buildbots I see
>
> [[[
>
> ..\..\..\subversion\tests\libsvn_fs_x\fs-x-pack-test.c:873,
>
> ..\..\..\subversion\libsvn_fs_x\batch_fsync.c:386,
>
> ..\..\..\subversion\libsvn_fs_x\batch_fsync.c:343,
>
> ..\..\..\subversion\libsvn_subr\io.c:3515: (apr_err=720005)
>
> svn_tests: E720005: Can't open file 'E:\svn-local\tests\subversion\tests\libsvn_fs_x':
Access is denied.
>
> FAIL:  fs-x-pack-test 13: test batch fsync
>
> ]]]
>
> after this commit
>
>
>
> It looks like the batch fsync introduced in this patch is trying to open a
> directory as a file?
>
> That is not going to work on Windows, and probably on more platforms.
> Opening a directory requires other functions.
>
> Note that this code is called inside a 'SVN_ON_POSIX' block, which I would
> assume shouldn't be active on Windows.
>
Yup, that's where the bug is / was. It must be "#if SVN_ON_POSIX"
instead of "#ifdef SVN_ON_POSIX". Fixed in r1694669.

> Last time we discussed batch syncing you told me/us, that you would use
> the original handles to perform the flush... This code appears to tell a
> different story?
>
No, it doesn't. The svn_fs_x__batch_fsync_new_path call only
informs the batch that this file is actually new, implying that the
parent folder needs fsync-ing on POSIX. When used as intended,
the file itself has been opened through / in the batch container.
Only if it hadn't, the container would need to open it.

> Reopening files may, and commonly will cause a huge performance hit on
> Windows... because virusscanners are designed to delay those operations,
> until they are sure the file doesn’t contain a virus; especially if the
> file was just written.
>
The FSX code should not reopen files. The tests might - just to
cover as many cases as they can.

-- Stefan^2.

Mime
View raw message