David Glasser wrote:
> My guess is that we need to rollback the transaction on error. So
> rewrite all the SVN_ERRs to be if/goto cleanup, or wrap commit_body in
> a little function like:
>
> svn_error_t *
> commit_exoskeleton(void *baton, apr_pool_t *pool)
> {
> struct commit_baton *cb = baton;
> fs_fs_data_t *ffd = cb->fs->fsap_data;
> svn_sqlite__db_t *db = ffb->rep_cache.db;
> svn_error_t *err;
>
> /* Start the sqlite transaction. */
> if (db)
> SVN_ERR(svn_sqlite__transaction_begin(db));
>
> err = commit_body(cb, pool);
>
> if (db)
> {
> if (err)
> svn_error_clear(svn_sqlite__transaction_rollback(db));
> else
> return svn_sqlite__transaction_commit(db);
> }
> return err;
> }
I was trying to figure out where the rollback should be, but ended up punting on
it, since I didn't know we had these kinds of failure modes. I'll take a look
at the breakage tomorrow.
-Hyrum
> On Tue, Nov 11, 2008 at 3:05 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>> FYI...
>>
>> Looks like this change broke something - see this buildbot result:
>> <http://www.mobsol.be/buildbot/win32-xp%2520VS2005/builds/1287>.
>>
>> >From the detailed log:
>>
>> svn_tests: cannot start a transaction within a transaction
>> FAIL: locks-test.exe 6: test that locking requires proper credentials
>> svn_tests: cannot start a transaction within a transaction
>> FAIL: locks-test.exe 7: test that locking is enforced in final commit step
>> svn_tests: cannot start a transaction within a transaction
>> FAIL: locks-test.exe 11: test that locks can expire
>>
>>
>> - Julian
>>
>>
>> On Tue, 2008-11-11 at 12:19 -0800, hwright_at_tigris.org wrote:
>>> Author: hwright
>>> Date: Tue Nov 11 12:19:26 2008
>>> New Revision: 34147
>>>
>>> Log:
>>> For FSFS rep-sharing: use SQLite transactions to ensure we don't commit bogus
>>> represenations to the rep cache.
>>>
>>> * subversion/libsvn_fs_fs/fs_fs.c
>>> (commit_body): Wrap with sqlite transaction handling.
>>>
>>> Modified:
>>> trunk/subversion/libsvn_fs_fs/fs_fs.c
>>>
>>> Modified: trunk/subversion/libsvn_fs_fs/fs_fs.c
>>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/fs_fs.c?pathrev=34147&r1=34146&r2=34147
>>> ==============================================================================
>>> --- trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Nov 11 11:46:27 2008 (r34146)
>>> +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Nov 11 12:19:26 2008 (r34147)
>>> @@ -5282,6 +5282,10 @@ commit_body(void *baton, apr_pool_t *poo
>>> apr_hash_t *txnprops;
>>> svn_string_t date;
>>>
>>> + /* Start the sqlite transaction. */
>>> + if (ffd->rep_cache.db)
>>> + SVN_ERR(svn_sqlite__transaction_begin(ffd->rep_cache.db));
>>> +
>>> /* Get the current youngest revision. */
>>> SVN_ERR(svn_fs_fs__youngest_rev(&old_rev, cb->fs, pool));
>>>
>>> @@ -5421,6 +5425,10 @@ commit_body(void *baton, apr_pool_t *poo
>>> /* Remove this transaction directory. */
>>> SVN_ERR(svn_fs_fs__purge_txn(cb->fs, cb->txn->id, pool));
>>>
>>> + /* Commit the sqlite transaction. */
>>> + if (ffd->rep_cache.db)
>>> + SVN_ERR(svn_sqlite__transaction_commit(ffd->rep_cache.db));
>>> +
>>> *cb->new_rev_p = new_rev;
>>>
>>> return SVN_NO_ERROR;
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
>> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>>
>>
>
>
>
Received on 2008-11-12 04:30:13 CET