Re: FAIL: locks-test 6/7/11 - Re: svn commit: r34147 - trunk/subversion/libsvn_fs_fs

Hyrum K. Wright wrote:
> 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.

r34160.

>> 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;