Re: svn commit: r34160 - trunk/subversion/libsvn_fs_fs

On Wed, Nov 12, 2008 at 12:32 PM, <hwright_at_tigris.org> wrote:
> Author: hwright
> Date: Wed Nov 12 12:32:17 2008
> New Revision: 34160
>
> Log:
> Followup to r34147 by rolling back the FSFS sqlite transaction on a failed
> commit.
>
> If at any point we decide that we have more places to used wrapped sqlite
> transactions, this could probably be generalized in the sqlite APIs.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (commit_body): Remove transaction handling.
> (commit_body_wrapper): New.
> (svn_fs_fs__commit): Call the new wrapper.
>
> 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=34160&r1=34159&r2=34160
> ==============================================================================
> --- trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Nov 12 12:09:48 2008 (r34159)
> +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Nov 12 12:32:17 2008 (r34160)
> @@ -5287,10 +5287,6 @@ 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));
>
> @@ -5430,15 +5426,42 @@ 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;
> }
>
> +/* Wrapper around commit_body() which implements SQLite transactions. Arguments
> + the same as commit_body().
> +
> + XXX: If we decide to wrap other stuff with sqlite transactions, this should
> + probably be generalized and moved into libsvn_subr/sqlite.c.
> + */
> +static svn_error_t *
> +commit_body_wrapper(void *baton, apr_pool_t *pool)
> +{
> + struct commit_baton *cb = baton;
> + fs_fs_data_t *ffd = cb->fs->fsap_data;
> + svn_error_t *err;
> +
> + /* Start the sqlite transaction. */
> + if (ffd->rep_cache.db)
> + SVN_ERR(svn_sqlite__transaction_begin(ffd->rep_cache.db));
> +
> + err = commit_body(baton, pool);
> +
> + /* Commit or rollback the sqlite transaction. */
> + if (ffd->rep_cache.db)
> + {
> + if (err)
> + SVN_ERR(svn_sqlite__transaction_rollback(ffd->rep_cache.db));

Hmm, you really think an error from the ROLLBACK command should
override the original error that caused you to issue the ROLLBACK?

--dave

> + else
> + SVN_ERR(svn_sqlite__transaction_commit(ffd->rep_cache.db));
> + }
> +
> + return err;
> +}
> +
> svn_error_t *
> svn_fs_fs__commit(svn_revnum_t *new_rev_p,
> svn_fs_t *fs,
> @@ -5450,7 +5473,7 @@ svn_fs_fs__commit(svn_revnum_t *new_rev_
> cb.new_rev_p = new_rev_p;
> cb.fs = fs;
> cb.txn = txn;
> - return svn_fs_fs__with_write_lock(fs, commit_body, &cb, pool);
> + return svn_fs_fs__with_write_lock(fs, commit_body_wrapper, &cb, pool);
> }
>
> svn_error_t *
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org