On Thu, Mar 17, 2011 at 4:21 PM, <rhuij...@apache.org> wrote:
> Author: rhuijben
> Date: Thu Mar 17 12:21:29 2011
> New Revision: 1082451
>
> URL: http://svn.apache.org/viewvc?rev=1082451&view=rev
> Log:
> On Windows avoid a file flush that is taking over 15% of the checkout time.
>
> * subversion/libsvn_subr/io.c
> (svn_io_write_unique): Avoid a very costly file flush on Windows.
>
[...]
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Mar 17 12:21:29 2011
> @@ -3199,8 +3199,17 @@ svn_io_write_unique(const char **tmp_pat
>
> err = svn_io_file_write_full(new_file, buf, nbytes, NULL, pool);
>
> - if (!err)
> + /* ### BH: Windows doesn't have the race condition between the write and
> the
> + ### rename that other operating systems might have. So allow windows
> + ### to decide when it wants to perform the disk synchronization
> using
> + ### the normal file locking and journaling filesystem rules.
> +
> + ### Note that this function doesn't handle the rename, so we aren't even
> + ### sure that we really have to sync. */
> +#ifndef WIN32
> + if (!err && nbytes > 0)
> err = svn_io_file_flush_to_disk(new_file, pool);
> +#endif
>
Hi Bert,
How did you come to conclusion that Windows doesn't have race
condition between write and rename?
It seems Windows actually have race condition between write and move
in case of unexpected computer shutdown. In this case move operation
completes after restart, but file contains zero data. I understand
that you intention was optimize client side operation but
svn_io_write_unique() function actively used in FSFS and lead
repository data corruption.
I've created small program to demonstrate this problem (see
attachment). On start it create four threads actively writing data to
disk and one thread that uses write + move to update file atomically.
If you run this program on virtual machine and then reset then with
very good chance you'll get 'current' file with zeros.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
// AtomicWrite.cpp : Defines the entry point for the console application.
//
#include <stdio.h>
#include <tchar.h>
#include <Windows.h>
#include <atlfile.h>
DWORD WINAPI WriteThreads(LPVOID name)
{
while(1)
{
CAtlFile file;
file.Create((LPCTSTR) name, GENERIC_WRITE, 0, CREATE_ALWAYS);
for(int i =0; i < 10000; i++)
{
char buf[1024];
memset(buf, 0x11, sizeof(buf));
file.Write(buf, sizeof(buf));
}
file.Flush();
file.Close();
Sleep(1);
}
}
void Verify(LPCWSTR name)
{
CAtlFile file;
if (file.Create((LPCTSTR) name, GENERIC_READ, 0, OPEN_EXISTING) == S_OK)
{
char buf[100];
DWORD read;
file.Read(buf, sizeof(buf), read);
if (read != 10)
{
wprintf(L"Invalid len %s\n", name);
exit(1);
}
for(int i = 0; i < read; i++)
{
if (buf[i] != i)
{
wprintf(L"Invalid data %s\n", name);
exit(1);
}
}
file.Close();
}
}
DWORD WINAPI MoveThread(LPVOID name)
{
while(1)
{
CAtlFile file;
file.Create(L"tmp", GENERIC_WRITE, 0, CREATE_ALWAYS);
char buf[10];
for(int i =0; i < sizeof(buf); i++)
{
buf[i] = i;
}
file.Write(buf, sizeof(buf));
// Uncomment to fix issue.
// file.Flush();
file.Close();
if (!MoveFileEx(L"tmp", L"current", MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED))
{
wprintf(L"move failed: %d\n", GetLastError());
exit(1);
}
Sleep(10);
}
}
int _tmain(int argc, _TCHAR* argv[])
{
Verify(L"current");
CreateThread(NULL, 0, WriteThreads, L"file1", 0, NULL);
CreateThread(NULL, 0, WriteThreads, L"file2", 0, NULL);
CreateThread(NULL, 0, WriteThreads, L"file3", 0, NULL);
CreateThread(NULL, 0, WriteThreads, L"file4", 0, NULL);
CreateThread(NULL, 0, MoveThread, L"current", 0, NULL);
getc(stdin);
return 0;
}
