[MERGE] Support streaming push to stacked branches.

Andrew Bennetts andrew.bennetts at canonical.com
Mon Feb 23 00:40:18 GMT 2009


Robert Collins wrote:
> This completes the addition of the RPC verb `Repository.insert_stream`,
> allowing streaming push to RemoteRepository objects which are stacked.
> 
> With this we're down to 60 round trips for push of a trivial branch, and
> 85 for a stacked trivial branch.

Once again, this looks good to me.

bb:approve

> +            # XXX: Ugly but important for correctness, *will* be fixed during
> +            # 1.13 cycle. Pushing a stream that is interrupted results in a
> +            # fallback to the _real_repositories sink *with a partial stream*.
> +            # Thats bad because we insert less data than bzr expected. To avoid
> +            # this we do a trial push to make sure the verb is accessible, and
> +            # do not fallback when actually pushing the stream. A cleanup patch
> +            # is going to look at rewinding/restarting the stream/partial
> +            # buffering etc.

A bit tangential to this patch, but that comment isn't quite right.  As
discussed on the phone this morning, partial streams aren't sent.

>          byte_stream = self._stream_to_byte_stream(stream, src_format)
> +        resume_tokens = ' '.join(self._resume_tokens)

Why not just transmit resume_tokens as a tuple?  HPSS protocol v3 will encode a
tuple-of-strings just fine for you (it bencodes its args).

> === modified file 'bzrlib/smart/repository.py'
[...]
> +            if missing_keys:
> +                # suspend the write group and tell the caller what we is
> +                # missing. We know we can suspend or else we would not have
> +                # entered this code path. (All repositories that can handle
> +                # missing keys can handle suspending a write group).
> +                write_group_tokens = self.repository.suspend_write_group()
> +                # bzip needed? missing keys should typically be a small set.
> +                # Should this be a streaming body response ?

Interesting questions.  I guess we wait and see how it works out in practice.

> === modified file 'bzrlib/tests/blackbox/test_push.py'
[...]
> -        self.assertEqual(61, rpc_count)
> +        self.assertEqual(60, rpc_count)
[...]
> -        self.assertEqual(101, rpc_count)
> +        self.assertEqual(85, rpc_count)

Yay :)




More information about the bazaar mailing list