I have started adjusting the pristine store access functions to
implement the spec in 'notes/wc-ng/pristine-store'. The attached patch
goes most of the way to sorting out the 'install' function, except that
testing for whether it's already installed is still done by looking on
disk for the file whereas it should look for the DB row. That's marked
with a '###' comment.
Other feedback on this will be much appreciated.
- Julian
### Patch in progress.
Make the WC's pristine text 'install' function safe for concurrent access,
according to the spec in 'notes/wc-ng/pristine-store'.
* subversion/include/private/svn_sqlite.h,
subversion/libsvn_subr/sqlite.c
(svn_sqlite__with_immediate_transaction): New function.
* subversion/libsvn_wc/wc_db_pristine.c
(pristine_install_baton, pristine_install_txn): New.
(svn_wc__db_pristine_install): Move the body of this function into
pristine_install_txn(), and call that inside a transaction with a
'RESERVED' lock.
* subversion/libsvn_wc/wc-queries.sql
(STMT_INSERT_PRISTINE): Don't ignore an attempt to add a row that already
exists; throws an error instead.
--This line, and those below, will be ignored--
Index: subversion/include/private/svn_sqlite.h
===================================================================
--- subversion/include/private/svn_sqlite.h (revision 1072006)
+++ subversion/include/private/svn_sqlite.h (working copy)
@@ -317,6 +317,14 @@ svn_sqlite__with_transaction(svn_sqlite_
svn_sqlite__transaction_callback_t cb_func,
void *cb_baton, apr_pool_t *scratch_pool);
+/* Like svn_sqlite__with_transaction(), but takes out a 'RESERVED' lock
+ immediately, instead of using the default deferred locking scheme. */
+svn_error_t *
+svn_sqlite__with_immediate_transaction(svn_sqlite__db_t *db,
+ svn_sqlite__transaction_callback_t cb_func,
+ void *cb_baton,
+ apr_pool_t *scratch_pool /* NULL allowed */);
+
/* Helper function to handle several SQLite operations inside a shared lock.
This callback is similar to svn_sqlite__with_transaction(), but can be
Index: subversion/libsvn_subr/sqlite.c
===================================================================
--- subversion/libsvn_subr/sqlite.c (revision 1072006)
+++ subversion/libsvn_subr/sqlite.c (working copy)
@@ -1063,6 +1063,48 @@ svn_sqlite__with_transaction(svn_sqlite_
}
svn_error_t *
+svn_sqlite__with_immediate_transaction(svn_sqlite__db_t *db,
+ svn_sqlite__transaction_callback_t cb_func,
+ void *cb_baton,
+ apr_pool_t *scratch_pool /* NULL allowed */)
+{
+ svn_error_t *err;
+
+ SVN_ERR(exec_sql(db, "BEGIN IMMEDIATE TRANSACTION;"));
+ err = cb_func(cb_baton, db, scratch_pool);
+
+ /* Commit or rollback the sqlite transaction. */
+ if (err)
+ {
+ svn_error_t *err2 = exec_sql(db, "ROLLBACK TRANSACTION;");
+
+ if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY)
+ {
+ int i;
+ /* See comments in svn_sqlite__with_transaction(). */
+
+ err2 = svn_error_compose_create(err2,
+ svn_error_create(SVN_ERR_SQLITE_RESETTING_FOR_ROLLBACK,
+ NULL, NULL));
+
+ for (i = 0; i < db->nbr_statements; i++)
+ if (db->prepared_stmts[i] && db->prepared_stmts[i]->needs_reset)
+ err2 = svn_error_compose_create(
+ err2,
+ svn_sqlite__reset(db->prepared_stmts[i]));
+
+ err2 = svn_error_compose_create(
+ exec_sql(db, "ROLLBACK TRANSACTION;"),
+ err2);
+ }
+
+ return svn_error_compose_create(err, err2);
+ }
+
+ return svn_error_return(exec_sql(db, "COMMIT TRANSACTION;"));
+}
+
+svn_error_t *
svn_sqlite__with_lock(svn_sqlite__db_t *db,
svn_sqlite__transaction_callback_t cb_func,
void *cb_baton,
Index: subversion/libsvn_wc/wc_db_pristine.c
===================================================================
--- subversion/libsvn_wc/wc_db_pristine.c (revision 1072006)
+++ subversion/libsvn_wc/wc_db_pristine.c (working copy)
@@ -34,6 +34,9 @@
#define PRISTINE_TEMPDIR_RELPATH ""
+/*
+ * See the spec in 'notes/wc-ng/pristine-store'.
+ */
/* Returns in PRISTINE_ABSPATH a new string allocated from RESULT_POOL,
holding the local absolute path to the file location that is dedicated
@@ -230,6 +233,63 @@ svn_wc__db_pristine_get_tempdir(const ch
}
+/* Data for pristine_install_txn(). */
+typedef struct pristine_install_baton
+{
+ /* The path to the source file that is to be moved into place. */
+ const char *tempfile_abspath;
+ /* The target path for the file, within the pristine store. */
+ const char *pristine_abspath;
+ /* The source file's SHA-1 checksum. */
+ const svn_checksum_t *sha1_checksum;
+ /* The source file's MD-5 checksum. */
+ const svn_checksum_t *md5_checksum;
+} pristine_install_baton;
+
+
+/* Install the pristine text described by BATON into the pristine store of
+ * SDB, or if it is already stored then just delete the new file
+ * BATON->tempfile_abspath. This implements spec section A-3(a). */
+static svn_error_t *
+pristine_install_txn(void *baton,
+ svn_sqlite__db_t *sdb,
+ apr_pool_t *scratch_pool)
+{
+ pristine_install_baton *b = baton;
+ apr_finfo_t finfo;
+ svn_sqlite__stmt_t *stmt;
+ svn_node_kind_t kind;
+
+ /* If this pristine text is already present in the store, just keep it:
+ * delete the new one and return. */
+ /* ### TODO: Check for the row existing. Checking for the existence of
+ * the file is not sufficient. */
+ SVN_ERR(svn_io_check_path(b->pristine_abspath, &kind, scratch_pool));
+ if (kind == svn_node_file)
+ {
+ /* Remove the tempfile, it's already there */
+ return svn_error_return(
+ svn_io_remove_file2(b->tempfile_abspath, FALSE, scratch_pool));
+ }
+
+ /* Put the file into its target location. */
+ SVN_ERR(svn_io_file_rename(b->tempfile_abspath, b->pristine_abspath,
+ scratch_pool));
+
+ SVN_ERR(svn_io_stat(&finfo, b->pristine_abspath, APR_FINFO_SIZE,
+ scratch_pool));
+
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
+ STMT_INSERT_PRISTINE));
+ SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, b->sha1_checksum, scratch_pool));
+ SVN_ERR(svn_sqlite__bind_checksum(stmt, 2, b->md5_checksum, scratch_pool));
+ SVN_ERR(svn_sqlite__bind_int64(stmt, 3, finfo.size));
+ SVN_ERR(svn_sqlite__insert(NULL, stmt));
+
+ return SVN_NO_ERROR;
+}
+
+
svn_error_t *
svn_wc__db_pristine_install(svn_wc__db_t *db,
const char *tempfile_abspath,
@@ -240,10 +300,7 @@ svn_wc__db_pristine_install(svn_wc__db_t
svn_wc__db_pdh_t *pdh;
const char *local_relpath;
const char *wri_abspath;
- const char *pristine_abspath;
- apr_finfo_t finfo;
- svn_sqlite__stmt_t *stmt;
- svn_node_kind_t kind;
+ struct pristine_install_baton b;
SVN_ERR_ASSERT(svn_dirent_is_absolute(tempfile_abspath));
SVN_ERR_ASSERT(sha1_checksum != NULL);
@@ -264,35 +321,20 @@ svn_wc__db_pristine_install(svn_wc__db_t
scratch_pool, scratch_pool));
VERIFY_USABLE_WCROOT(pdh->wcroot);
- SVN_ERR(get_pristine_fname(&pristine_abspath, pdh->wcroot->abspath,
+ b.tempfile_abspath = tempfile_abspath;
+ b.sha1_checksum = sha1_checksum;
+ b.md5_checksum = md5_checksum;
+
+ SVN_ERR(get_pristine_fname(&b.pristine_abspath, pdh->wcroot->abspath,
sha1_checksum,
TRUE /* create_subdir */,
scratch_pool, scratch_pool));
-
- SVN_ERR(svn_io_check_path(pristine_abspath, &kind, scratch_pool));
-
- if (kind == svn_node_file)
- {
- /* Remove the tempfile, it's already there */
- return svn_error_return(
- svn_io_remove_file2(tempfile_abspath,
- FALSE, scratch_pool));
- }
-
- /* Put the file into its target location. */
- SVN_ERR(svn_io_file_rename(tempfile_abspath, pristine_abspath,
- scratch_pool));
-
- SVN_ERR(svn_io_stat(&finfo, pristine_abspath, APR_FINFO_SIZE,
- scratch_pool));
-
- SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
- STMT_INSERT_PRISTINE));
- SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, sha1_checksum, scratch_pool));
- SVN_ERR(svn_sqlite__bind_checksum(stmt, 2, md5_checksum, scratch_pool));
- SVN_ERR(svn_sqlite__bind_int64(stmt, 3, finfo.size));
- SVN_ERR(svn_sqlite__insert(NULL, stmt));
+ /* Ensure the SQL txn has at least a 'RESERVED' lock before we start looking
+ * at the disk, to ensure no concurrent pristine install/delete txn. */
+ SVN_ERR(svn_sqlite__with_immediate_transaction(pdh->wcroot->sdb,
+ pristine_install_txn, &b,
+ scratch_pool));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_wc/wc-queries.sql
===================================================================
--- subversion/libsvn_wc/wc-queries.sql (revision 1072006)
+++ subversion/libsvn_wc/wc-queries.sql (working copy)
@@ -427,7 +427,7 @@ SELECT id, work FROM work_queue ORDER BY
DELETE FROM work_queue WHERE id = ?1;
-- STMT_INSERT_PRISTINE
-INSERT OR IGNORE INTO pristine (checksum, md5_checksum, size, refcount)
+INSERT INTO pristine (checksum, md5_checksum, size, refcount)
VALUES (?1, ?2, ?3, 0);
-- STMT_SELECT_PRISTINE_MD5_CHECKSUM
