Philip Martin <philip.martin_at_wandisco.com> writes:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>
>> Philip Martin wrote on Tue, 13 Apr 2010 at 13:08 +0100:
>>> Summary: FSFS hotcopy doesn't handle SQLite databases properly, this
>>> affects the new revprop packing db and the 1.6 rep sharing db.
>>
>> revprops.db consists of a single table; perhaps we could take advantage of
>> this to duplicate it "by hand", until the sqlite API has stabilized?
>>
>> (IOW, perhaps the hotcopy logic could be tailored to revprops.db, without
>> having logic generic enough to be able to duplicate any live sqlite DB)
>
> You mean use SQL statements: SELECT from the source table, iterate
> over the rows doing INSERT INTO the destination table. That's
> probably less efficient than a plain copy but should be reliable.
OK, I tried this and it's slow. I copied an 11MB rep cache db (85961
rows) into an otherwise empty repository. Hotcopy using 1.6 takes
about 0.25s on my machine. With the patch below to use SQL statements
it's nearly 3s (all the inserts are inside one transaction, before I
did that it was taking over 17s). That's an order of magnitude slower
which could be a problem, the SELECT on the source will block writes
to the database during the copy.
* subversion/libsvn_fs_fs/rep-cache.h
(svn_fs_fs__copy_rep_cache): New.
* subversion/libsvn_fs_fs/rep-cache.c
(svn_fs_fs__copy_rep_cache): New.
(open_rep_cache): Copied from svn_fs_fs__open_rep_cache with parameters
that don't include svn_fs_t. Add comment about lack of thread safety.
(svn_fs_fs__open_rep_cache): Now just a wrapper.
* subversion/libsvn_fs_fs/rep-cache-db.sql
(STMT_GET_ALL_REPS): New.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__hotcopy): Use new copy interface, copy rep cache before the
rev files.
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 933881)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1498,6 +1498,13 @@
/* Copy the config. */
SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_CONFIG, pool));
+ /* Copy the rep cache before copying the rev files to make sure all
+ cached references will be present in the copy. */
+ src_subdir = svn_dirent_join(src_path, REP_CACHE_DB_NAME, pool);
+ SVN_ERR(svn_io_check_path(src_subdir, &kind, pool));
+ if (kind == svn_node_file)
+ SVN_ERR(svn_fs_fs__copy_rep_cache(src_path, dst_path, pool));
+
/* Copy the min unpacked rev, and read its value. */
if (format >= SVN_FS_FS__MIN_PACKED_FORMAT)
{
@@ -1658,12 +1665,6 @@
PATH_NODE_ORIGINS_DIR, TRUE, NULL,
NULL, pool));
- /* Now copy the rep cache. */
- src_subdir = svn_dirent_join(src_path, REP_CACHE_DB_NAME, pool);
- SVN_ERR(svn_io_check_path(src_subdir, &kind, pool));
- if (kind == svn_node_file)
- SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, REP_CACHE_DB_NAME, pool));
-
/* Copy the txn-current file. */
if (format >= SVN_FS_FS__MIN_TXN_CURRENT_FORMAT)
SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_TXN_CURRENT, pool));
Index: subversion/libsvn_fs_fs/rep-cache-db.sql
===================================================================
--- subversion/libsvn_fs_fs/rep-cache-db.sql (revision 933881)
+++ subversion/libsvn_fs_fs/rep-cache-db.sql (working copy)
@@ -40,6 +40,11 @@
where hash = ?1;
+-- STMT_GET_ALL_REPS
+select hash, revision, offset, size, expanded_size
+from rep_cache;
+
+
-- STMT_SET_REP
insert into rep_cache (hash, revision, offset, size, expanded_size)
values (?1, ?2, ?3, ?4, ?5);
Index: subversion/libsvn_fs_fs/rep-cache.c
===================================================================
--- subversion/libsvn_fs_fs/rep-cache.c (revision 933881)
+++ subversion/libsvn_fs_fs/rep-cache.c (working copy)
@@ -37,28 +37,30 @@
REP_CACHE_DB_SQL_DECLARE_STATEMENTS(statements);
-
-svn_error_t *
-svn_fs_fs__open_rep_cache(svn_fs_t *fs,
- apr_pool_t *pool)
+static svn_error_t*
+open_rep_cache(fs_fs_data_t *ffd,
+ const char *fs_path,
+ apr_pool_t *fs_pool,
+ apr_pool_t *scratch_pool)
{
- fs_fs_data_t *ffd = fs->fsap_data;
const char *db_path;
int version;
- /* Be idempotent. */
+ /* Be idempotent. ### This is not thread safe. */
if (ffd->rep_cache_db)
return SVN_NO_ERROR;
/* Open (or create) the sqlite database. It will be automatically
closed when fs->pool is destoyed. */
- db_path = svn_dirent_join(fs->path, REP_CACHE_DB_NAME, pool);
+ db_path = svn_dirent_join(fs_path, REP_CACHE_DB_NAME, scratch_pool);
SVN_ERR(svn_sqlite__open(&ffd->rep_cache_db, db_path,
svn_sqlite__mode_rwcreate, statements,
0, NULL,
- fs->pool, pool));
+ fs_pool, scratch_pool));
- SVN_ERR(svn_sqlite__read_schema_version(&version, ffd->rep_cache_db, pool));
+ SVN_ERR(svn_sqlite__read_schema_version(&version, ffd->rep_cache_db,
+ scratch_pool));
+ /* ### This is not thread safe. */
if (version < REP_CACHE_SCHEMA_FORMAT)
{
/* Must be 0 -- an uninitialized (no schema) database. Create
@@ -71,6 +73,14 @@
}
svn_error_t *
+svn_fs_fs__open_rep_cache(svn_fs_t *fs,
+ apr_pool_t *pool)
+{
+ return svn_error_return(open_rep_cache(fs->fsap_data, fs->path,
+ fs->pool, pool));
+}
+
+svn_error_t *
svn_fs_fs__get_rep_reference(representation_t **rep,
svn_fs_t *fs,
svn_checksum_t *checksum,
@@ -169,3 +179,63 @@
return svn_sqlite__insert(NULL, stmt);
}
+
+struct copy_rep_cache_baton {
+ svn_sqlite__db_t *src_sdb;
+};
+
+static svn_error_t *
+copy_rep_cache(void *baton, svn_sqlite__db_t *dst_sdb, apr_pool_t *scratch_pool)
+{
+ struct copy_rep_cache_baton *b = baton;
+
+ svn_sqlite__stmt_t *get_stmt, *set_stmt;
+
+ SVN_ERR(svn_sqlite__get_statement(&get_stmt, b->src_sdb, STMT_GET_ALL_REPS));
+ SVN_ERR(svn_sqlite__get_statement(&set_stmt, dst_sdb, STMT_SET_REP));
+
+ while (1)
+ {
+ svn_boolean_t have_row;
+ SVN_ERR(svn_sqlite__step(&have_row, get_stmt));
+ if (!have_row)
+ break;
+ SVN_ERR(svn_sqlite__bindf(set_stmt, "siiii",
+ svn_sqlite__column_text(get_stmt, 0, NULL),
+ svn_sqlite__column_int64(get_stmt, 1),
+ svn_sqlite__column_int64(get_stmt, 2),
+ svn_sqlite__column_int64(get_stmt, 3),
+ svn_sqlite__column_int64(get_stmt, 4)));
+ SVN_ERR(svn_sqlite__insert(NULL, set_stmt));
+ }
+
+ SVN_ERR(svn_sqlite__reset(get_stmt));
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_fs_fs__copy_rep_cache(const char *src_path,
+ const char *dst_path,
+ apr_pool_t *scratch_pool)
+{
+ fs_fs_data_t *src_ffd = apr_pcalloc(scratch_pool, sizeof(*src_ffd));
+ fs_fs_data_t *dst_ffd = apr_pcalloc(scratch_pool, sizeof(*src_ffd));
+ struct copy_rep_cache_baton baton;
+
+ SVN_ERR(open_rep_cache(src_ffd, src_path, scratch_pool, scratch_pool));
+ SVN_ERR(open_rep_cache(dst_ffd, dst_path, scratch_pool, scratch_pool));
+
+ baton.src_sdb = src_ffd->rep_cache_db;
+
+ SVN_ERR(svn_sqlite__with_transaction(dst_ffd->rep_cache_db, copy_rep_cache,
+ &baton, scratch_pool));
+
+ /* These handles can't be reached from outside this function and so
+ can't be used further, close them immediately and avoid waiting
+ for pool cleanup. */
+ SVN_ERR(svn_sqlite__close(src_ffd->rep_cache_db));
+ SVN_ERR(svn_sqlite__close(dst_ffd->rep_cache_db));
+
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_fs_fs/rep-cache.h
===================================================================
--- subversion/libsvn_fs_fs/rep-cache.h (revision 933881)
+++ subversion/libsvn_fs_fs/rep-cache.h (working copy)
@@ -62,6 +62,13 @@
svn_boolean_t reject_dup,
apr_pool_t *pool);
+/* Copy the rep cache database from the FS in SRC_PATH to the FS in
+ DST_PATH. */
+svn_error_t *
+svn_fs_fs__copy_rep_cache(const char *src_path,
+ const char *dst_path,
+ apr_pool_t *pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
--
Philip
Received on 2010-04-14 13:18:37 CEST