You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2012/11/21 11:07:16 UTC

Re: svn commit: r1411982 - in /subversion/branches/1.6.x: ./ STATUS subversion/libsvn_client/commit_util.c subversion/libsvn_fs_fs/fs_fs.c

Bert Huijben wrote on Wed, Nov 21, 2012 at 10:36:43 +0100:
> 
> 
> > -----Original Message-----
> > From: svn-role@apache.org [mailto:svn-role@apache.org]
> > Sent: woensdag 21 november 2012 05:02
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1411982 - in /subversion/branches/1.6.x: ./ STATUS
> > subversion/libsvn_client/commit_util.c subversion/libsvn_fs_fs/fs_fs.c
> > 
> > Author: svn-role
> > Date: Wed Nov 21 04:01:41 2012
> > New Revision: 1411982
> > 
> > URL: http://svn.apache.org/viewvc?rev=1411982&view=rev
> > Log:
> > Reintegrate the 1.6.x-rep_write_cleanup branch:
> > 
> >  * r1403964, r1403982, r1410106, r1410203
> >    Make fs_fs properly cleanup after a failed transmission of a representation.
> >    Justification:
> >      Read errors can create problems for users of the WANdisco replicator
> >      which does retry requests.  Can result in garbage representations in the
> >      rev file.
> >    Notes:
> 
> 
> 
> >      Branch is required since our client code needs a small tweak to deal
> >      with pool lifetimes to make the fix work properly with ra_local.  We'd
> >      made a similar change with wcng in 1.7 already.
> 
> 
> What would be the effect of *not* patching the client?
> 

Not destroying iterpool would cause rep_write_cleanup() (pool cleanup
handler) not to fire [according to breser], hence not call
unlock_proto_rev(), hence not clear the "being_written" flag in the txn
object.  That object lives in the fs_fs_shared_data_t (which outlives
svn_fs_t handles) so subsequent attempts to commit that transaction
within the same process before clearing the pool would spuriously fail.

> What impact does this have on other users of the fs/repos apis?
> 
> 
> In other words: Is this a breaking change that we shouldn't port back to 1.6 clients and try to patch server only?
> 
> 	Bert 
> 

Re: svn commit: r1411982 - in /subversion/branches/1.6.x: ./ STATUS subversion/libsvn_client/commit_util.c subversion/libsvn_fs_fs/fs_fs.c

Posted by Ben Reser <be...@reser.org>.
On Wed, Nov 21, 2012 at 2:07 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Not destroying iterpool would cause rep_write_cleanup() (pool cleanup
> handler) not to fire [according to breser], hence not call
> unlock_proto_rev(), hence not clear the "being_written" flag in the txn
> object.  That object lives in the fs_fs_shared_data_t (which outlives
> svn_fs_t handles) so subsequent attempts to commit that transaction
> within the same process before clearing the pool would spuriously fail.

This is true of the fsfs change but the client change is not actually
required.  See my lengthier email in response to Bert.

Re: svn commit: r1411982 - in /subversion/branches/1.6.x: ./ STATUS subversion/libsvn_client/commit_util.c subversion/libsvn_fs_fs/fs_fs.c

Posted by Ben Reser <be...@reser.org>.
On Wed, Nov 21, 2012 at 2:07 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Not destroying iterpool would cause rep_write_cleanup() (pool cleanup
> handler) not to fire [according to breser], hence not call
> unlock_proto_rev(), hence not clear the "being_written" flag in the txn
> object.  That object lives in the fs_fs_shared_data_t (which outlives
> svn_fs_t handles) so subsequent attempts to commit that transaction
> within the same process before clearing the pool would spuriously fail.

This is true of the fsfs change but the client change is not actually
required.  See my lengthier email in response to Bert.