You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/02/10 03:58:45 UTC
Re: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
On Mon, Feb 9, 2009 at 12:52, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Mon Feb 9 12:52:35 2009 (r35771)
> @@ -5800,31 +5800,35 @@ commit_body(void *baton, apr_pool_t *poo
> return SVN_NO_ERROR;
> }
>
> -/* Wrapper around commit_body() which implements SQLite transactions. Arguments
> - the same as commit_body().
> +/* Baton for use with an sqlite transaction'd commit body. */
> +struct commit_sqlite_txn_baton
> +{
> + struct commit_baton *cb;
> + apr_pool_t *pool;
> +};
You could simplify quite a bit if you just put a pool into the
commit_baton structure.
>...
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1132049
RE: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: dinsdag 10 februari 2009 4:59
> To: dev@subversion.tigris.org
> Subject: Re: svn commit: r35771 - in trunk/subversion: include/private
> libsvn_fs_fs libsvn_subr
>
> On Mon, Feb 9, 2009 at 12:52, Hyrum K. Wright <hy...@hyrumwright.org>
> wrote:
> >...
> > +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Mon Feb 9 12:52:35
> 2009 (r35771)
> > @@ -5800,31 +5800,35 @@ commit_body(void *baton, apr_pool_t *poo
> > return SVN_NO_ERROR;
> > }
> >
> > -/* Wrapper around commit_body() which implements SQLite
> transactions. Arguments
> > - the same as commit_body().
> > +/* Baton for use with an sqlite transaction'd commit body. */
> > +struct commit_sqlite_txn_baton
> > +{
> > + struct commit_baton *cb;
> > + apr_pool_t *pool;
> > +};
>
> You could simplify quite a bit if you just put a pool into the
> commit_baton structure.
Hyrum,
Why don't you just rollback transactions on pool cleanup?
You could just mark them as already committed with some new function, and
automatically rollback when the pool is cleaned up before it is committed
(or when it is rolled back before).
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1132852
RE: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Hyrum K. Wright [mailto:hyrum_wright@mail.utexas.edu]
> Sent: woensdag 11 februari 2009 15:18
> To: Bert Huijben
> Cc: 'Greg Stein'; dev@subversion.tigris.org
> Subject: Re: svn commit: r35771 - in trunk/subversion: include/private
> libsvn_fs_fs libsvn_subr
>
> The SQLite db closed on pool cleanup, which rolls back any pending
> transactions. The transaction rollback is handled inside SQLite--we
> don't have to do anything special.
Thanks,
I was looking at an older version of libsvn_subr/sqlite.c that still asked
for explicit cleanup :(
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1139201
Re: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Feb 11, 2009, at 6:10 AM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: woensdag 11 februari 2009 11:54
>> To: Bert Huijben
>> Cc: hyrum_wright@mail.utexas.edu; dev@subversion.tigris.org
>> Subject: Re: svn commit: r35771 - in trunk/subversion: include/
>> private
>> libsvn_fs_fs libsvn_subr
>>
>> Ugh. That would be a very hard-to-see control flow. Weird little
>> untracable side effects.
>
> In the normal flow (no errors) you would just commit or rollback; it
> would
> only add a defined rollback behavior on pool cleanup for the case
> where it
> wasn't before.
>
> I don't see how that would make things harder than your previous
> suggestions
> on moving a file on pool cleanup, etc.
>
> And in many cases you would have a sub or iterpool that has the same
> lifetime as the transaction, so I can't think of any real surprises..
>
> Just... when anything unexpected goes wrong... it would also give a
> defined
> behavior to external code that clears the apr pools on a
> SVN_ERR_MALFUNCTION_NO_RETURN() that is turned in a C++ exception or
> something.
>
>
> Crashing Visual Studio, Eclipse or anything else that might contain
> hours of
> work, on just a serf operation that receives some garbage data (or
> some
> null reference in another layer) is not an option in many cases.
>
> And I don't know what our current behavior on open sqlite sessions
> is on
> pool cleanups.
>
>
> (At first glance it looks like it just stops looking at sqlite..
> keeping
> current transactions open... and makes sqlite block other
> transactions from
> this and other processes).
The SQLite db closed on pool cleanup, which rolls back any pending
transactions. The transaction rollback is handled inside SQLite--we
don't have to do anything special.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1139120
RE: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: woensdag 11 februari 2009 11:54
> To: Bert Huijben
> Cc: hyrum_wright@mail.utexas.edu; dev@subversion.tigris.org
> Subject: Re: svn commit: r35771 - in trunk/subversion: include/private
> libsvn_fs_fs libsvn_subr
>
> Ugh. That would be a very hard-to-see control flow. Weird little
> untracable side effects.
In the normal flow (no errors) you would just commit or rollback; it would
only add a defined rollback behavior on pool cleanup for the case where it
wasn't before.
I don't see how that would make things harder than your previous suggestions
on moving a file on pool cleanup, etc.
And in many cases you would have a sub or iterpool that has the same
lifetime as the transaction, so I can't think of any real surprises..
Just... when anything unexpected goes wrong... it would also give a defined
behavior to external code that clears the apr pools on a
SVN_ERR_MALFUNCTION_NO_RETURN() that is turned in a C++ exception or
something.
Crashing Visual Studio, Eclipse or anything else that might contain hours of
work, on just a serf operation that receives some garbage data (or some
null reference in another layer) is not an option in many cases.
And I don't know what our current behavior on open sqlite sessions is on
pool cleanups.
(At first glance it looks like it just stops looking at sqlite.. keeping
current transactions open... and makes sqlite block other transactions from
this and other processes).
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1138626
Re: svn commit: r35771 - in trunk/subversion: include/private
libsvn_fs_fs libsvn_subr
Posted by Greg Stein <gs...@gmail.com>.
Ugh. That would be a very hard-to-see control flow. Weird little
untracable side effects.
On Tue, Feb 10, 2009 at 00:14, Bert Huijben <b....@competence.biz> wrote:
>
>
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: dinsdag 10 februari 2009 4:59
>> To: dev@subversion.tigris.org
>> Subject: Re: svn commit: r35771 - in trunk/subversion: include/private
>> libsvn_fs_fs libsvn_subr
>>
>> On Mon, Feb 9, 2009 at 12:52, Hyrum K. Wright <hy...@hyrumwright.org>
>> wrote:
>> >...
>> > +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Mon Feb 9 12:52:35
>> 2009 (r35771)
>> > @@ -5800,31 +5800,35 @@ commit_body(void *baton, apr_pool_t *poo
>> > return SVN_NO_ERROR;
>> > }
>> >
>> > -/* Wrapper around commit_body() which implements SQLite
>> transactions. Arguments
>> > - the same as commit_body().
>> > +/* Baton for use with an sqlite transaction'd commit body. */
>> > +struct commit_sqlite_txn_baton
>> > +{
>> > + struct commit_baton *cb;
>> > + apr_pool_t *pool;
>> > +};
>>
>> You could simplify quite a bit if you just put a pool into the
>> commit_baton structure.
>
> Hyrum,
>
> Why don't you just rollback transactions on pool cleanup?
>
> You could just mark them as already committed with some new function, and
> automatically rollback when the pool is cleaned up before it is committed
> (or when it is rolled back before).
>
> Bert
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1138421