Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Daniel Shahaf writes: > Philip Martin wrote on Thu, Jan 13, 2011 at 10:56:09 +: >> Stefan Sperling writes: >> > The scenario we're talking about is where people run a 1.7 server >> > and use a 1.6 or earlier svnsync binary. >> > >> > I suppose in this case your hook script would help? >> >> No, I don't think it does. Daniel's change introduced new protocol >> elements to get the new atomic behaviour, a old client using the old >> protocol still gets the old non-atomic behaviour. > > We're talking about the svn_repos_fs_change_rev_prop4() changes, not > about the RA protocol changes. > > Briefly, svn_repos_fs_change_rev_prop4() (which is the function that > computes the ACTION parameter to the pre-revprop-change hook) uses the > *atomic* FS "change revprop" API regardless of the client version. Ah! So we have changed the behaviour of the server for old clients. I believe that means that the ACTION parameter is now reliable within the hook, which means that the script should work as a solution when using a 1.7 server with older clients. In a mixed environment the script is also necessary to prevent old clients stealing the lock from well-behaved 1.7 clients. -- Philip
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Philip Martin wrote on Thu, Jan 13, 2011 at 10:56:09 +: > Stefan Sperling writes: > > The scenario we're talking about is where people run a 1.7 server > > and use a 1.6 or earlier svnsync binary. > > > > I suppose in this case your hook script would help? > > No, I don't think it does. Daniel's change introduced new protocol > elements to get the new atomic behaviour, a old client using the old > protocol still gets the old non-atomic behaviour. We're talking about the svn_repos_fs_change_rev_prop4() changes, not about the RA protocol changes. Briefly, svn_repos_fs_change_rev_prop4() (which is the function that computes the ACTION parameter to the pre-revprop-change hook) uses the *atomic* FS "change revprop" API regardless of the client version.
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
On Thu, Jan 13, 2011 at 10:56:09AM +, Philip Martin wrote: > Stefan Sperling writes: > > The scenario we're talking about is where people run a 1.7 server > > and use a 1.6 or earlier svnsync binary. > > > > I suppose in this case your hook script would help? > > No, I don't think it does. Daniel's change introduced new protocol > elements to get the new atomic behaviour, a old client using the old > protocol still gets the old non-atomic behaviour. Right. I'll adjust the release notes then. Thanks. > We could change the behaviour associated with the old protocol, and then > we could backport that relatively small change to 1.6, and that would > allow the script to work. That would be the regression/improvement I > mentioned. I don't think that's necessary. Backporting this fix would give people one less reason to upgrade to 1.7 :) Anyone suffering from the race condition in 1.6 can use the known workaround (run svnsync jobs on one computer and synchronise the processes via file locks). Stefan
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Stefan Sperling writes: > On Thu, Jan 13, 2011 at 09:31:36AM +, Philip Martin wrote: >> Daniel Shahaf writes: >> >> > * The difference that's supposed to cause Philip's script to work as >> > advertised is that svn_repos_fs_change_rev_prop4() uses the same >> > propvalue to compute the ACTION parameter and to pass as the "old >> > propvalue" to the FS for atomicity. >> > >> > I'm too sleepy right now to determine whether Philip's script will >> > actually work as advertised given this server-side change, so just >> > throwing it out there for now. >> >> The server side change hasn't been applied to the 1.6 branch AFAIK. >> That means that the ACTION parameter in the hook script is not reliable >> and so the script doesn't fix the problem. >> >> The obstacle to applying the server side fix to the 1.6 branch is that >> it changes the behaviour of server and so could be considered a >> regression, although that same behaviour change could also be considered >> an improvement. > > The scenario we're talking about is where people run a 1.7 server > and use a 1.6 or earlier svnsync binary. > > I suppose in this case your hook script would help? No, I don't think it does. Daniel's change introduced new protocol elements to get the new atomic behaviour, a old client using the old protocol still gets the old non-atomic behaviour. We could change the behaviour associated with the old protocol, and then we could backport that relatively small change to 1.6, and that would allow the script to work. That would be the regression/improvement I mentioned. > If not, the current draft of the 1.7 release notes will need to be adjusted. -- Philip
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
On Thu, Jan 13, 2011 at 09:31:36AM +, Philip Martin wrote: > Daniel Shahaf writes: > > > * The difference that's supposed to cause Philip's script to work as > > advertised is that svn_repos_fs_change_rev_prop4() uses the same > > propvalue to compute the ACTION parameter and to pass as the "old > > propvalue" to the FS for atomicity. > > > > I'm too sleepy right now to determine whether Philip's script will > > actually work as advertised given this server-side change, so just > > throwing it out there for now. > > The server side change hasn't been applied to the 1.6 branch AFAIK. > That means that the ACTION parameter in the hook script is not reliable > and so the script doesn't fix the problem. > > The obstacle to applying the server side fix to the 1.6 branch is that > it changes the behaviour of server and so could be considered a > regression, although that same behaviour change could also be considered > an improvement. The scenario we're talking about is where people run a 1.7 server and use a 1.6 or earlier svnsync binary. I suppose in this case your hook script would help? If not, the current draft of the 1.7 release notes will need to be adjusted. Stefan
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Daniel Shahaf writes: > * The difference that's supposed to cause Philip's script to work as > advertised is that svn_repos_fs_change_rev_prop4() uses the same > propvalue to compute the ACTION parameter and to pass as the "old > propvalue" to the FS for atomicity. > > I'm too sleepy right now to determine whether Philip's script will > actually work as advertised given this server-side change, so just > throwing it out there for now. The server side change hasn't been applied to the 1.6 branch AFAIK. That means that the ACTION parameter in the hook script is not reliable and so the script doesn't fix the problem. The obstacle to applying the server side fix to the 1.6 branch is that it changes the behaviour of server and so could be considered a regression, although that same behaviour change could also be considered an improvement. -- Philip
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
s...@apache.org wrote on Wed, Jan 12, 2011 at 20:36:34 -: > Author: stsp > Date: Wed Jan 12 20:36:33 2011 > New Revision: 1058308 > > URL: http://svn.apache.org/viewvc?rev=1058308&view=rev > Log: > * publish/docs/release-notes/1.7.html > (atomic-revprops): Mention that this is also a client-side fix (suggested by > danielsh). Provide a link to the pre-revprop-change hook script by > philip which fixes the issue #3546 race even for pre-1.7 svnsync clients. > Two comments: * The entire locking discussion applies unmodified to svnrdump, since it shares the locking logic with svnsync: namely, 'svnrdump load' is susceptible to the same race condition with ≤1.6 servers. That logic is now svn_ra__get_operational_lock(). (Its docstring doesn't currently mention the race condition with old servers; I'll get to it.) * The difference that's supposed to cause Philip's script to work as advertised is that svn_repos_fs_change_rev_prop4() uses the same propvalue to compute the ACTION parameter and to pass as the "old propvalue" to the FS for atomicity. I'm too sleepy right now to determine whether Philip's script will actually work as advertised given this server-side change, so just throwing it out there for now. > Modified: > subversion/site/publish/docs/release-notes/1.7.html > > Modified: subversion/site/publish/docs/release-notes/1.7.html > URL: > http://svn.apache.org/viewvc/subversion/site/publish/docs/release-notes/1.7.html?rev=1058308&r1=1058307&r2=1058308&view=diff > == > --- subversion/site/publish/docs/release-notes/1.7.html (original) > +++ subversion/site/publish/docs/release-notes/1.7.html Wed Jan 12 20:36:33 > 2011 > @@ -551,16 +551,21 @@ a more useful error message on temporary > > > > -Atomic revprop changes (server) > +Atomic revprop changes (client and server) > title="Link to this section">¶ > > > -Revprop changes are now handled atomically on the server. > +Revprop changes are now handled atomically. > This fixes a known race condition in the locking algorithm used by > svnsync (see href="http://subversion.tigris.org/issues/show_bug.cgi?id=3546"; > - >issue #3546.) > + >issue #3546.). > +With a 1.7 server, it is possible to fix the svnsync race condition > +even for pre-1.7 svnsync clients by installing the pre-revprop-change hook > +script given in > +http://subversion.tigris.org/issues/show_bug.cgi?id=3546#desc12"; > + >comment 12 of issue #3546. > > > > >
