On 16 June 2015 at 22:57, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> wrote:
> Hey there,
>
> One of the links recently provided by Daniel Klima pointed
> to a way to enable write caching even on USB devices.
> So, I could use my Windows installation for experiments now
> without the risk of brick-ing 2 grand worth of disks by pulling
> the plug tens of times.
>
>
> TL;DR
> =====
> FlushFileBuffers operates on whole files, not just the parts
> written through the respective handle. Not calling it after rename
> results in potential data loss. Calling it after rename eliminates
> the problem at least in most cases.
>
> Setup:
> =====
> I used the attached program to conduct 3 different experiments,
> each trying a specific modification / fsync sequence. All would
> write to an USB stick which had OS write cache enabled for it
> in Windows 7.
>
> All tests run an unlimited number of iterations - until there is an
> I/O error (e.g. caused by disconnecting out the drive). For each
> run, separate files and different file contents will being written
> ("run number xyz", repeated many times). So, we can determine
> which file contents is complete and correct and whether all files
> are present. Each successful iteration is logged to the console.
> We expect the data for all these to be complete.
>
> The stick got yanked out at a random point in time, reconnected
> after about a minute, chkdsk /f run on it and then the program
> output would be compared with the USB stick's content.
I've tried to repeat your tests, but I failed to do that:
1. Your attached program miss some scripts around to perform real tests.
2. I don't have the same USB stick that you used in your tests :)
Also I don't think that NTFS on removable flash USB drive could be
used to simulate powerloss scenario on Windows: removable disks are
not available during the system boot, so Windows cannot replay NTFS
journal during startup.
Instead of this I tweaked 'repos-test 25' to emulate concurrent
commits of 10kb files in 4 parallel threads (see attached very dirty
patch).
Then I've performed several tests on Windows Server 2012 R2 running
VMWare Workstation 9 forcing power off after 300-400 commits. I've
performed 10x tests and never get repository corruption even when I
removed FlushFileBuffers() call *after* rename. During the restart the
OS may report that it recovered volume data, but after that the
repository data remain in the consistent state. Removing other
FlushFileBuffers() calls, results repository corruption after two
runs.
While I agree that passing MOVEFILE_COPY_ALLOWED to MoveFileEx() is a
bug, but calling FlushFileBuffers() is not necessary at least in case
of NTFS on permanently connected disk. I suppose it happens because
MoveFileEx() already journaled which means that journal is flushed to
disk before operation completes. But we may add MOVEFILE_WRITE_THROUGH
flag to make sure that this operation will be synced on other
filesystems or network shares, but this require more Windows specific
code.
---
Ivan Zhakov
Index: subversion/libsvn_fs_fs/util.c
===================================================================
--- subversion/libsvn_fs_fs/util.c (revision 1687052)
+++ subversion/libsvn_fs_fs/util.c (working copy)
@@ -644,13 +644,13 @@
/* APR will *not* error out on Win32 if this requires a copy instead of
of a move. */
SVN_ERR(svn_io_file_rename(old_filename, new_filename, pool));
-
+#if 0
/* Flush the target of the copy to disk. */
SVN_ERR(svn_io_file_open(&file, new_filename, APR_WRITE,
APR_OS_DEFAULT, pool));
SVN_ERR(svn_io_file_flush_to_disk(file, pool));
SVN_ERR(svn_io_file_close(file, pool));
-
+#endif
/* Copying permissions is a no-op on WIN32. */
#else
Index: subversion/tests/libsvn_repos/repos-test.c
===================================================================
--- subversion/tests/libsvn_repos/repos-test.c (revision 1687052)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -23,6 +23,7 @@
#include <stdlib.h>
#include <string.h>
#include <apr_pools.h>
+#include <apr_thread_proc.h>
#include "../svn_test.h"
@@ -3612,9 +3613,132 @@
return SVN_NO_ERROR;
}
+struct commit_file_baton_t {
+ const char *repos_path;
+ const char *fs_path;
+ apr_pool_t *pool;
+ svn_error_t *err;
+ apr_thread_t *tid;
+ int num;
+};
+
+void fs_warning(void *baton, svn_error_t *err)
+{
+ svn_handle_warning(stderr, err);
+}
+
+svn_error_t *
+commit_file_child_body(struct commit_file_baton_t *baton, apr_pool_t *pool)
+{
+ svn_repos_t *repos;
+ svn_revnum_t rev;
+ apr_pool_t *iterpool;
+
+ SVN_ERR(svn_repos_open(&repos, baton->repos_path, pool));
+ svn_fs_set_warning_func(svn_repos_fs(repos), fs_warning, NULL);
+ iterpool = svn_pool_create(pool);
+
+ rev = 0;
+ while (TRUE)
+ {
+ svn_fs_txn_t *txn;
+ svn_fs_root_t *root;
+ svn_stringbuf_t *buf;
+ int i;
+
+ svn_pool_clear(iterpool);
+
+ SVN_ERR(svn_repos_fs_begin_txn_for_commit(&txn, repos, rev,
+ "author", "log", iterpool));
+
+ SVN_ERR(svn_fs_txn_root(&root, txn, iterpool));
+ if (rev == 0)
+ {
+ SVN_ERR(svn_fs_make_file(root, baton->fs_path, iterpool));
+ }
+
+ buf = svn_stringbuf_create_ensure(100*100, iterpool);
+ for (i = 0; i < 100; i++)
+ {
+ svn_stringbuf_appendfill(buf, rand() % 200 + 1, 100);
+ }
+
+ SVN_ERR(svn_test__set_file_contents(root, baton->fs_path,
+ buf->data, iterpool));
+
+ SVN_ERR(svn_repos_fs_commit_txn(NULL, repos, &rev, txn, iterpool));
+
+ fprintf(stderr, "Thread %d: Committed %ld\n", baton->num, rev);
+ }
+
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+}
+
+static void * APR_THREAD_FUNC
+commit_file_child(apr_thread_t *tid, void *data)
+{
+ struct commit_file_baton_t *baton = data;
+
+ baton->err = commit_file_child_body(baton, baton->pool);
+ if (baton->err)
+ svn_handle_error2(baton->err, stderr, FALSE, "test:");
+ svn_pool_destroy(baton->pool);
+ apr_thread_exit(tid, 0);
+ return NULL;
+}
+
+
+static svn_error_t *
+test_concurrent_commits(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_repos_t *repos;
+ int i;
+ struct commit_file_baton_t threads[4];
+
+ /* Create test repository. */
+ SVN_ERR(svn_test__create_repos(&repos,
+ "test-repo-concurrent-commits",
+ opts, pool));
+
+ for (i = 0; i < sizeof(threads) / sizeof(threads[0]); i++)
+ {
+ apr_status_t status;
+ apr_threadattr_t *tattr;
+ threads[i].pool = svn_pool_create(pool);
+ threads[i].err = SVN_NO_ERROR;
+ threads[i].repos_path = "test-repo-concurrent-commits";
+ threads[i].fs_path = apr_psprintf(pool, "/file-%d", i);
+ threads[i].num = i;
+
+ status = apr_threadattr_create(&tattr, pool);
+ if (status)
+ return svn_error_wrap_apr(status, "Can't create threadattr");
+
+ status = apr_thread_create(&threads[i].tid, tattr, commit_file_child,
&threads[i], pool);
+ if (status)
+ return svn_error_wrap_apr(status, "Can't create thread");
+ }
+
+ for (i = 0; i < sizeof(threads) / sizeof(threads[0]); i++)
+ {
+ apr_status_t status;
+ apr_status_t child_status;
+ status = apr_thread_join(&child_status, threads[i].tid);
+ if (status)
+ return svn_error_wrap_apr(status, "Can't join thread");
+
+ if (threads[i].err)
+ return svn_error_trace(threads[i].err);
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* The test table. */
-static int max_threads = 4;
+static int max_threads = 1;
static struct svn_test_descriptor_t test_funcs[] =
{
@@ -3667,6 +3791,8 @@
"test test_repos_fs_type"),
SVN_TEST_OPTS_PASS(deprecated_access_context_api,
"test deprecated access context api"),
+ SVN_TEST_OPTS_PASS(test_concurrent_commits,
+ "perform concurrent commits"),
SVN_TEST_NULL
};
