[MERGE] repository write groups

[Robert Collins]

On Fri, 2007-07-20 at 12:46 -0400, John Arbash Meinel wrote:
> John Arbash Meinel has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
> I like the idea of write groups, though it seems a little bit redundant 
> with our current Transaction code.
> I believe the idea is that you want to get rid of our "lock + implicit 
> commit at unlock" and make it more of a "start + explicit commit and 
> finish | explicit rollback with abort".
> 
> 
> +        try:
> +            # don't need a specific exception for now - this is
> +            # really to be sure it's used right, not for signalling
> +            # semantic information.
> +            self.assertRaises(errors.BzrError, repo.start_write_group)
> +        finally:
> 
> ^- I think you *do* want an explicit (AlreadyInWriteGroup) or some other 
> clear error.

I can add one, but it seems silly when the only user of it will be the
test suite; exception classes really are for catchers of exceptions.

> It isn't quite clear to me who gets to decide when to start the write 
> group, since now you are having the transaction start/end be 
> non-reentrant. Are you trying to make low-level functions just assert 
> that we are in a write group, and high-level ones must take a write 
> group?

these are the mutators of repositories that I know of:
 - fetch
 - CommitBuilder
 - uncommit(in future?)
 - gc/reconcile
 - pack
 - sign_revision

These, and only these, will take out write groups at the start of an
operation and release them at the end of an operation.

Anything else - e.g. 'I'm writing to knit X now' is forbidden at the
repository API level - the repository is /not/ a store of blobs like in
git, so allowing arbitrary poking at the internals is harmful. Our
repository is a structured interface around the above writing
operations. This interface looks friendly to svn/hg/git plugins,
friendly to the current knit disk format, and is friendly to a pack
based format too.

> The only problem with that, is that I believe there are some code paths 
> which are called directly *and* are called as children functions from 
> some other main-level action.
> 
> I can't guarantee that, but I know when I was trying to fix the progress 
> bars for 'bzr branch' and 'bzr pull' I ran into the fact that they 
> execute the same 'fetch()' process, but with a different number of 
> layers above it. So getting the progress right for one of them, meant 
> that the other one exposed too much, or too little information.

Well, the patch is working in my repository branch, so I assert its
fine :).

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070730/6ad76f19/attachment.pgp