Hello.
I think I've found a problem with the rep-cache.db verification. If a commit
occurs during the rep-cache.db verification, this can lead to a post-commit
error "database is locked" and new representations will not be added in the
rep-cache.db. The problem occurs because during the verification all cache
entries are retrieved using one large query with holding a sqlite lock, that
prevents adding new cache entries.
The problem can occur for other cases where rep-cache.db is changed, such as
during 'svnadmin build rep-cache'.
I propose to retrieve cache entries in batches and process them without holding
a sqlite lock. I checked that the new approach does not affect the verify
performance and assume that it should work correctly in the case of concurrent
working with the rep-cache.db.
Please see the attached patch. More technical details can be found in the log
message.
Regards,
Denis Kovalchuk
Fix "database is locked" error when committing during rep-cache.db
verification.
If a commit occurs during the rep-cache.db verification, this can lead to a
post-commit error "database is locked" and new representations will not be
added in the rep-cache.db. The problem occurs because during the verification
all cache entries are retrieved using one large query with holding a sqlite
lock, that prevents adding new cache entries.
As a solution, select cache entries in batches of 1000 and process them without
holding a sqlite lock. The size of the batch is based on the result of the
following benchmark:
Verifying rep-cache.db which contains 1.5 million entries:
1) batch size 1: ~132 sec.
2) batch size 10: ~66 sec.
3) batch size 1000: ~59 sec.
4) All entries at one time (trunk behavior): ~59 sec.
* subversion/libsvn_fs_fs/rep-cache-db.sql
(STMT_GET_REPS_FOR_RANGE): Add a filter by hash and LIMIT. The hash is used
as an offset in the table and provides a fast lookup, because it is a
primary key in the table.
* subversion/libsvn_fs_fs/rep-cache.c
(svn_fs_fs__walk_rep_reference): Select data from the rep-cache.db in batches
of 1000 entries, construct and store representations into an array and
process them without holding a sqlite lock. The last received hash is used
as an offset in the table for the next query. Check for cancellation only
when processing the received batch, to avoid calling the cancellation
callback while holding an sqlite lock.
Patch by: Denis Kovalchuk <denis.kovalc...@visualsvn.com>
Index: subversion/libsvn_fs_fs/rep-cache-db.sql
===================================================================
--- subversion/libsvn_fs_fs/rep-cache-db.sql (revision 1876224)
+++ subversion/libsvn_fs_fs/rep-cache-db.sql (working copy)
@@ -68,7 +68,9 @@ VALUES (?1, ?2, ?3, ?4, ?5)
/* Works for both V1 and V2 schemas. */
SELECT hash, revision, offset, size, expanded_size
FROM rep_cache
-WHERE revision >= ?1 AND revision <= ?2
+WHERE revision >= ?1 AND revision <= ?2 AND hash > ?3
+ORDER BY hash
+LIMIT ?4
-- STMT_GET_MAX_REV
/* Works for both V1 and V2 schemas. */
Index: subversion/libsvn_fs_fs/rep-cache.c
===================================================================
--- subversion/libsvn_fs_fs/rep-cache.c (revision 1876224)
+++ subversion/libsvn_fs_fs/rep-cache.c (working copy)
@@ -173,9 +173,12 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
void *cancel_baton,
apr_pool_t *pool)
{
+ static const int batch_size = 1000;
fs_fs_data_t *ffd = fs->fsap_data;
svn_sqlite__stmt_t *stmt;
svn_boolean_t have_row;
+ svn_stringbuf_t *sha1_digest;
+ apr_array_header_t *representations;
int iterations = 0;
apr_pool_t *iterpool = svn_pool_create(pool);
@@ -200,57 +203,77 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
SVN_ERR(svn_fs_fs__ensure_revision_exists(max, fs, iterpool));
}
+ /* Use an empty string for hash to start from the beginning of the table. */
+ sha1_digest = svn_stringbuf_create_empty(pool);
+ representations = apr_array_make(pool, batch_size, sizeof(representation_t));
+
SVN_ERR(svn_sqlite__get_statement(&stmt, ffd->rep_cache_db,
STMT_GET_REPS_FOR_RANGE));
- SVN_ERR(svn_sqlite__bindf(stmt, "rr",
- start, end));
- /* Walk the cache entries. */
- SVN_ERR(svn_sqlite__step(&have_row, stmt));
- while (have_row)
+ while (1)
{
- representation_t *rep;
- const char *sha1_digest;
- svn_error_t *err;
- svn_checksum_t *checksum;
+ int i;
- /* Clear ITERPOOL occasionally. */
- if (iterations++ % 16 == 0)
- svn_pool_clear(iterpool);
+ svn_pool_clear(iterpool);
- /* Check for cancellation. */
- if (cancel_func)
+ apr_array_clear(representations);
+
+ SVN_ERR(svn_sqlite__bindf(stmt, "rrsd", start, end,
+ sha1_digest->data, batch_size));
+ SVN_ERR(svn_sqlite__step(&have_row, stmt));
+ /* Walk the cache entries. */
+ while (have_row)
{
- err = cancel_func(cancel_baton);
+ representation_t *rep;
+ svn_error_t *err;
+ svn_checksum_t *checksum;
+
+ /* Clear ITERPOOL occasionally. */
+ if (iterations++ % 16 == 0)
+ svn_pool_clear(iterpool);
+
+ /* Construct a representation_t. */
+ rep = apr_array_push(representations);
+ memset(rep, 0, sizeof(*rep));
+ svn_fs_fs__id_txn_reset(&rep->txn_id);
+ svn_stringbuf_set(sha1_digest, svn_sqlite__column_text(stmt, 0,
NULL));
+ err = svn_checksum_parse_hex(&checksum, svn_checksum_sha1,
+ sha1_digest->data, iterpool);
if (err)
return svn_error_compose_create(err, svn_sqlite__reset(stmt));
+
+ rep->has_sha1 = TRUE;
+ memcpy(rep->sha1_digest, checksum->digest, sizeof(rep->sha1_digest));
+ rep->revision = svn_sqlite__column_revnum(stmt, 1);
+ rep->item_index = svn_sqlite__column_int64(stmt, 2);
+ rep->size = svn_sqlite__column_int64(stmt, 3);
+ rep->expanded_size = svn_sqlite__column_int64(stmt, 4);
+
+ SVN_ERR(svn_sqlite__step(&have_row, stmt));
}
- /* Construct a representation_t. */
- rep = apr_pcalloc(iterpool, sizeof(*rep));
- svn_fs_fs__id_txn_reset(&rep->txn_id);
- sha1_digest = svn_sqlite__column_text(stmt, 0, iterpool);
- err = svn_checksum_parse_hex(&checksum, svn_checksum_sha1,
- sha1_digest, iterpool);
- if (err)
- return svn_error_compose_create(err, svn_sqlite__reset(stmt));
+ /* Reset a statement to release a database lock. */
+ SVN_ERR(svn_sqlite__reset(stmt));
- rep->has_sha1 = TRUE;
- memcpy(rep->sha1_digest, checksum->digest, sizeof(rep->sha1_digest));
- rep->revision = svn_sqlite__column_revnum(stmt, 1);
- rep->item_index = svn_sqlite__column_int64(stmt, 2);
- rep->size = svn_sqlite__column_int64(stmt, 3);
- rep->expanded_size = svn_sqlite__column_int64(stmt, 4);
+ if (representations->nelts <= 0)
+ break;
- /* Walk. */
- err = walker(rep, walker_baton, fs, iterpool);
- if (err)
- return svn_error_compose_create(err, svn_sqlite__reset(stmt));
+ /* Walk the representations. */
+ for (i = 0; i < representations->nelts; i++)
+ {
+ representation_t *rep = &APR_ARRAY_IDX(representations, i,
representation_t);
- SVN_ERR(svn_sqlite__step(&have_row, stmt));
+ /* Clear ITERPOOL occasionally. */
+ if (iterations++ % 16 == 0)
+ svn_pool_clear(iterpool);
+
+ if (cancel_func)
+ SVN_ERR(cancel_func(cancel_baton));
+
+ SVN_ERR(walker(rep, walker_baton, fs, iterpool));
+ }
}
- SVN_ERR(svn_sqlite__reset(stmt));
svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
