HPSS status from the NL sprint...

[Andrew Bennetts]

On Wed, Feb 07, 2007 at 12:58:10PM +1100, Robert Collins wrote:
> Just thought I'd drop everyone a status note from the recent sprint in
> the Netherlands.
> 
> First a thank you to everyone who turned up, and Wouter [who I unfairly
> blame as organising the sprint] :).
> 
> The hpss is making strides, but theres still a bunch of work to be done.
> 
> The way I see it, we have three phases:
>  * all tests passing
>  * key use cases optimised
>  * 'real objects' support removed - all native methods.

This plan sounds good to me.

> We can merge some of the hpss branch to bzr.dev once the first phase is
> complete, *but* we may choose not to - depending on how nice its
> looking :). I think that we should aim to get to phase one this coming
> week.

I'd love to get the tests on that branch passing ASAP.  It's a real pain working
on a branch where >100 tests failing is the norm, and I imagine it's an even
larger barrier for potential new contributors to that branch.

I think next time I'd try harder to have a branch that's always closer to a
state that could be acceptable for merging to bzr.dev.

> As far as I can tell, the following things are all that are needed to
> get to phase 1:
>  * RemoteBranch paramterisation. At the moment the
> branch_implementations tests dont run test for RemoteBranch. This should
> be quite straight forward to hook up, requiring the same changes done to
> the repository_implementations logic.

I already did this a couple of days ago.  Try "bzr selftest
branch_implementations.*Remote" for example.

>  * LockDir reuse. Because the server runs small transactions, but we
> want to hold locks open longer, we need a way to tell the server that
> the lock held on e.g. the Branch, is the one we created, and also that
> it should be able to attach to that lock. This is quite easily done by
> having lockdir be able to provide a token for the lock, and extending
> the lock_write methods on Branch and Repository to allow passing in a
> token. Finally we need the RemoteBranch and RemoteRepository objects to
> acquire this token and pass it around as needed. I think for efficiency
> it should look something like [pseudo code]:
>   branch_token, repository_token = self.client.call('lock this branch')
>   if self._real_branch is not None:
>       self._real_branch.lock_write(branch_token=branch_token,
> repository_token=repository_token)
> 
> That is, that the smart server protocol should perform the locking, and
> no individual lock steps should cross the wire.
> 
> We probably need to allow this call to include an already acquired
> repository token, for when we have two branches pointing at the same
> repository, and lock both.

Is this actually needed to make tests pass?  Why wouldn't providing the
existing lock_read/lock_write methods over the smart protocol be enough? 

I don't feel like I have a very solid understanding of the problem here.  Is
there a test case demonstrating the problem?

>  * RemoteBranch and RemoteRepository lock context caching. Currently
> there is no caching of data within a transaction for these classes,
> which there is for real objects. We should wire this up - see
> LockableFiles for the core of the logic - and then use it judiciously.
> An example is getting the branch config file - thats a round trip every
> time branch.get_config() is called, and its not needed. (in this case,
> its currently also a problem for regular branches, but most things are
> not:).

This sounds like a performance issue, not a correctness one.  Is this related to
getting passing tests, or merely nice to have?

>  * Slug through the tests. This is basically ensuring there is either a
> real_object based implementation, or adding a verb to the smart server
> to fast-path a specific request. This is very easy now, with the
> refactoring we did during the sprint.

I'd in fact be inclined to freeze adding new verbs in favour of focussing on
getting tests passing.  At the moment I think there's a great risk that
incidental breakage in adding these new features might not be noticed quickly
because of the dire state of the test suite.

> Please dig right in - the shared branch is
> sftp://bazaar.launchpad.net/~bzr/bzr/hpss. We'll do a full review for
> this when it lands in bzr.dev, so dont stress about code review at the
> moment : but if you want a review, just send a regular mail to the
> list ;).

And please ask on the list or IRC if you get stuck trying to work on this — it's
a large branch, and parts might not be as clear as they should be, but if you
ask someone ought to be able to help explain it and make it clearer.

-Andrew.