[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: atomic revprop commits at the RA level

From: David Glasser <glasser_at_mit.edu>
Date: 2006-08-11 20:42:57 CEST

On 8/9/06, David Glasser <glasser@mit.edu> wrote:

> So is the former API clean enough to be worth implementing, or is the
> latter not too invasive, or is there some other option that I'm
> completely missing?

I decided to give this a shot. Attached is a patch which begins to
implement it; the big obvious lack right now is that it only works
with ra_local and ra_svnserve. This is by far the biggest subversion
patch I've worked on, so I figure it's best to stick it out for review
as soon as possible.


* I recognize that the thread about this has not really settled on a
  choice of API, and some folks support adding this capability
  to the svn_delta_editor_t type. Arguably neither way of doing it is
  really clean: either you pollute the delta description with extra
  properties, or you allow one (and only one) RA API to be used in the
  middle of the editor stream. I currently prefer the latter, since
  the other way would require upgrading about 50 APIs that use
  svn_delta_editor_t, and this one just requires one new API and one
  API documentation change (relaxing a "don't do this" constraint).
  (Well, and indirectly two other API bumps.)

  However, I seriously do not have much ego invested in this
  particular way of doing things, and if committers want to say
  "thanks for the work, but we don't like the API", then, well, that's
  how it is. It's been pretty educational anyway.

* Do I need to put anything in libsvn_ra/wrapper_template.h? That is,
  do I need to support the use of a brand new RA API in the old
  ra_plugin conventions?

* Similarly, do I have to support the use of a brand new svnserve command
  under non-pipelined editing?

* With ra_svn, the only way that you can find out that the remote
repository doesn't
  support set-rev-prop is by trying it and getting an error back. As
far as I can tell this
  is how the locking commands work too. Presumably users of this API who want
  to be able to work against older servers will need to explicitly
trap the "command
  not recognized" error.

* An earlier version of this patch didn't create
  svn_repos_get_commit_editor5 and instead had each of its relevant
  invokers manually create and deal with aborting the txn. This was a
  huge pain, since I basically ended up needing to copy-and-paste code from
  libsvn_repos to create the txns and then create wrapper editors
  to deal with abortion; this effort was duplicated in both RA implementations.
  I think the svn_repos_get_commit_editor5 API is saner than trying to simulate
  it without it existing.

  It does create a slight behavior change, though it's still
  consistent with the API's documentation: the managed txn is now made
  in svn_repos_get_commit_editorN itself instead of waiting until
  open_root is called. Is this a problem?

* I changed svnsync to use the new API, mostly for testing
  purposes. The version in this patch does *not* fall back to
  non-atomic revprop copying if the server doesn't support it, since
  for debugging purposes it's helpful to know that the server isn't
  supporting it yet. I will certainly add that to a final patch.

* No support in any of the WebDAV-related modules. I am not
  particularly familiar with any of them; I'd be happy if somebody who
  was wanted to write their implementations, but also could certainly
  dive in myself.

* Tested this using the ra_local C tests and svnsync. Currently passes
"make check".

New svn_ra_set_revprop_for_commit API, which allows atomically setting
revprops at initial commit time over the RA layer.

* subversion/include/svn_ra.h
  (svn_ra_set_revprop_for_commit): New RA API.
  (svn_ra_get_commit_editor2): Document that it's OK to call the
   new RA API during an editor session.

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_set_revprop_for_commit): Implement vtable dispatch for new
   RA API.

* subversion/libsvn_ra/ra_load.h
  (svn_ra__vtable_t): Add slot for new RA API.

* subversion/include/svn_error_codes.h
  (SVN_ERR_RA_SVN_INVALID_SET_REVPROP): New error for use of
   set-rev-prop command in a non-committing editor session.

* subversion/include/svn_repos.h
  (svn_repos_get_commit_editor5): API upgrade to allow the caller
   to request access to a repos-managed txn.

* subversion/include/svn_ra_svn.h
  (svn_ra_svn_drive_editor2): API upgrade to give driver access to

* subversion/libsvn_ra_local/ra_plugin.c
  (svn_ra_local__get_commit_editor): Get txn from repos commit editor.
  (svn_ra_local__set_revprop_for_commit): Implement new RA API.
  (ra_local_vtable): Add implementation of new RA API to vtable.

* subversion/libsvn_ra_local/ra_local.h
  (svn_ra_local__session_baton_t): Add slot for txn.

* subversion/tests/libsvn_ra_local/ra-local-test.c
  (set_revprops_atomically, commit_callback): Add new test
   which tests new RA API on ra_local (and for that matter
   tests commits).
  (test_funcs): Call new test.

* subversion/svnsync/main.c
  (copy_revprops_atomically): Like copy_revprops, but uses new RA API;
   used when copying a revision for the first time.
  (do_synchronize): Use copy_revprops_atomically instead of copy_revprops
   when you can.

* subversion/libsvn_repos/commit.c
  (open_root): Move managed txn creation from here to ...
  (svn_repos_get_commit_editor5): ... here, which is an upgraded
   API which can return the managed txn.

* subversion/libsvn_ra_svn/client.c
  (ra_svn_set_revprop_for_commit): Implement new RA API.
  (ra_svn_vtable): Add implementation of new RA API to vtable.

* subversion/libsvn_ra_svn/protocol:
  Document new set-rev-prop editor command.

* subversion/libsvn_ra_svn/editor.c
  (svn_ra_svn_drive_editor2): Upgrade API to accept txn.

* subversion/libsvn_ra_svn/editorp.c
  (ra_svn_driver_state_t): Add slot for txn.
  (ra_svn_handle_set_revprop_for_commit): Handle new RA API.
  (ra_svn_edit_cmds): Add handler for new RA API.
  (svn_ra_svn__drive_editorp): Store the given txn in the
   driver state.

* subversion/libsvn_ra_svn/ra_svn.h
  (svn_ra_svn__drive_editorp): Take an extra txn argument.

* subversion/svnserve/serve.c
  (commit): Get txn from repos_get_commit_editor5 and pass it
   through to svn_ra_svn_drive_editor2.

David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Received on Fri Aug 11 20:43:48 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.