[PATCH] changegroup: allow sending snapshot deltas in cg2

[Michael Edgar]

Sorry for taking a while to get back to this thread - I wanted to take
a big step
back before committing to any one direction.

On Tue, Nov 25, 2014 at 6:58 PM, Matt Mackall <mpm at selenic.com> wrote:
>
> On Tue, 2014-11-25 at 13:17 -0800, Pierre-Yves David wrote:
> >
> > On 11/21/2014 05:28 PM, Matt Mackall wrote:
> > > On Fri, 2014-11-21 at 17:53 -0500, Mike Edgar wrote:
> > >> # HG changeset patch
> > >> # User Mike Edgar <adgar at google.com>
> > >> # Date 1416610038 18000
> > >> #      Fri Nov 21 17:47:18 2014 -0500
> > >> # Node ID ce1d4cdad3e2a324198a348f5a62f86e9b0e1a73
> > >> # Parent  a179db3db9b96b38c10c491e6e7e7ad5f40a7787
> > >> changegroup: allow sending snapshot deltas in cg2
> > >>
> > >> The changegroup2 format allows each revision to be sent with
> > >> a configurable base. That base is presently restricted to
> > >> p1, p2, or the previous revision in the revlog. By allowing
> > >> a null base, we can send snapshot deltas when it is efficient
> > >> to do so.
> > >
> > > Not sure what you mean by efficient here. We generally assume that
> > > bandwidth is more scarce than CPU, so calculating a new delta is
> > > generally preferred to sending a full revision. This seems to prefer the
> > > opposite trade-off?
> >
> > This change comes to support the "Censored" nodes effort. The censored
> > node cannot be used as delta base (and would likely be inefficient
> > anyway) so we have an alternative. It some case it would be an easy but
> > suboptinal solution to issue a full delta. In some other it will be the
> > only available option.
>
> This doesn't get me any closer to answering the above concerns.

I agree with Matt, this change is wrong. I came upon the "no snapshot"
limitation in the course of implementing censorship exchange, but too quickly
assumed lifting the "no snapshot" restriction would be appropriate in
non-censor cases too.

The revlog might not have a deltaparent because there is actually no suitable
delta base globally, but there are purely local reasons it might have stored a
snapshot. Recent changes to limit delta chain length are one example (hg:
76effa770ff9). Variation in compression algorithms are another.

This change trusts the revlog whenever it has no delta base, but the revlog
only provides a *hint* as to a suitable delta base, and changegroup/exchange
ignores that hint if it's unhelpful in the exchange context. For example, if
it can't prove the recipient has the delta base, it ignores the hint. I agree
with Matt that the bandwidth/CPU tradeoff is also an important reason to
ignore the hint.

.
As for the censor line of work: I have a new approach that I'm writing up
and will share on a Wiki page early next week. It will include a channel for
signaling that a revision is tombstoned without decoding the revision's
text. This new signal can ultimately be considered in code paths like this
one.





-- 
Michael Edgar | Software Engineer | adgar at google.com | 518-496-6958