Re: [PATCH] protocol upload-pack-v2

5 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] protocol upload-pack-v2

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:19

Junio C Hamano [off-list ref] writes:
I have a feeling that it is a bit too premature to specify the
details at such a low level as "capaiblities are announced by
prefixing four-byte 'c', 'a', 'p', ':' in front" and "a multi-record
group has its element count at the beginning (or an end marker at
the end, for that matter)", and it may be a better idea to outline
all the necessary elements at a bit higher level first---that would
avoid needs for useless exchanges like what we are having right now.

....  If you keep the
discussion at the level like "fetch first asks capabilities it wants
upload-pack-2 to enable, optionally gives the current shallow
boundaries when the capaibilty says the other side supports it, and
then starts showing what it has" while we are trying to achieve
concensus on what kind of protocol elements we would need, and what
information each element would carry, the discussion will help us
reach a shared understanding on what to write down in EBNF form
exactly faster, I would imagine.
And I see we went silent after this, so let's try to stir the pot
again to see if it simmers.

This is a follow-up on $gmane/264553, which is a continuation of
$gmane/264000, but instead of giving two required readings to
readers, I'll start with reproduction of the two, and add a few more
things the current protocol lacks that I would want to see in the
updated protocol.



The current protocol has the following problems that limit us:

 - It is not easy to make it resumable, because we recompute every
   time.  This is especially problematic for the initial fetch aka
   "clone" as we will be talking about a large transfer. Redirection
   to a bundle hosted on CDN might be something we could do
   transparently.

 - The protocol extension has a fairly low length limit.

 - Because the protocol exchange starts by the server side
   advertising all its refs, even when the fetcher is interested in
   a single ref, the initial overhead is nontrivial, especially when
   you are doing a small incremental update.  The worst case is an
   auto-builder that polls every five minutes, even when there is no
   new commits to be fetched.

 - Because we recompute every time, taking into account of what the
   fetcher has, in addition to what the fetcher obtained earlier
   from us in order to reduce the transferred bytes, the payload for
   incremental updates become tailor-made for each fetch and cannot
   be easily reused.

 - The semantics of the side-bands are unclear.

   - Is band #2 meant only for progress output (I think the current
     protocol handlers assume that and unconditionally squelch it
     under --quiet)?  Do we rather want a dedicated "progress" and
     "error message" sidebands instead?

   - Is band #2 meant for human consumption, or do we expect the
     other end to interpret and act on it?  If the former, would it
     make sense to send locale information from the client side and
     ask the server side to produce its output with _("message")?

 - The semantics of packet_flush() is suboptimal, and this
   shortcoming seeps through to the protocol mapped to the
   smart-HTTP transport.

   Originally, packet_flush() was meant as "Here is an end of one
   logical section of what I am going to speak.", hinting that it
   might be a good idea for the underlying implementation to hold
   the packets up to that point in-core and then write(2) them all
   out (i.e. "flush") to the file descriptor only when we handle
   packet_flush().  It never meant "Now I am finished speaking for
   now and it is your turn to speak."

   But because HTTP is inherently a ping-pong protocol where the
   requestor at one point stops talking and lets the responder
   speak, the code to map our protocol to the smart HTTP transport
   made the packet_flush() boundary as "Now I am done talking, it is
   my turn to listen."

   We probably need two kinds of packet_flush().  When a requestor
   needs to say two or more logical groups of things before telling
   the other side "Now I am done talking; it is your turn.", we need
   some marker (i.e. the original meaning of packet_flush()) at the
   end of these logical groups.  And in order to be able to say "Now
   I am done saying everything I need to say at this point for you
   to respond to me.  It is your turn.", we need another kind of
   marker.

 - The fetch-pack direction does the common-parent discovery but the
   push-pack direction does not.  This is OK for the normal
   fast-forward push, in which case we will see a known commit on
   the tip of the branch we are pushing into, but makes forced push
   inefficient.

 - The existing common-parent discovery done on the fetch-pack side
   enumerates commits contiguously traversing the history to the
   past.  We might want to go exponential or Fibonacci to quickly
   find an ancient common commit and bisect the history from there
   (or it might turn out not to be worth it).

 - We may want to revamp the builtin/receive-pack.c::report() that
   reports the final result of a push back to the pusher to tell the
   exact object name that sits at the updated tips of refs, not just
   refnames.  It will allow the server side to accept a push of
   commit X to a branch, do some "magic" on X (e.g. rebase it on top
   of the current tip, merge it with the current tip, or let a hook
   to rewrite the commit in any way it deems appropriate) and put
   the resulting commit Y at the tip of the branch.  Without such a
   revamp, it is currently not possible to sensibly allow the server
   side to rewrite what got pushed.

 - If we were to start allowing the receiver side to rewrite pushed
   commits, the updated send-pack protocol must be able to send the
   new objects created by that "magic" back to the pusher.  The
   current protocol does not allow the receive-pack to send packdata
   back to send-pack.

I'd like to see a new protocol that lets us overcome the above
limitations (did I miss others? I am sure people can help here)
sometime this year.

Re: [PATCH] protocol upload-pack-v2

From: Duy Nguyen <hidden>
Date: 2016-06-15 23:04:20

On Wed, Apr 1, 2015 at 2:58 AM, Junio C Hamano [off-list ref] wrote:
This is a follow-up on $gmane/264553, which is a continuation of
$gmane/264000, but instead of giving two required readings to
readers, I'll start with reproduction of the two, and add a few more
things the current protocol lacks that I would want to see in the
updated protocol.
I think the important thing to get v2 started is making sure we do not
need v3 to get rid of any of these limitations. In other words v2
should be extensible enough to implement them later. I'm looking from
this perspective.
The current protocol has the following problems that limit us:

 - It is not easy to make it resumable, because we recompute every
   time.  This is especially problematic for the initial fetch aka
   "clone" as we will be talking about a large transfer. Redirection
   to a bundle hosted on CDN might be something we could do
   transparently.
Sending multiple packs or some redirection instructions could be done
even with v1. The only recompute part that is unavoidable in v1 is ref
advertisement, which I think is solved.
 - The protocol extension has a fairly low length limit.
One pkt-line per protocol extension should do it.
 - Because the protocol exchange starts by the server side
   advertising all its refs, even when the fetcher is interested in
   a single ref, the initial overhead is nontrivial, especially when
   you are doing a small incremental update.  The worst case is an
   auto-builder that polls every five minutes, even when there is no
   new commits to be fetched.
One of the reason v2 is started, should be ok with current v2 design.
 - Because we recompute every time, taking into account of what the
   fetcher has, in addition to what the fetcher obtained earlier
   from us in order to reduce the transferred bytes, the payload for
   incremental updates become tailor-made for each fetch and cannot
   be easily reused.
Well, we reuse at a lower level, pack-objects would try to copy
existing deltas instead of making new ones. We can cache new deltas in
hope that they may be useful for the next fetch. But that has nothing
to do with the protocol..
 - The semantics of the side-bands are unclear.

   - Is band #2 meant only for progress output (I think the current
     protocol handlers assume that and unconditionally squelch it
     under --quiet)?  Do we rather want a dedicated "progress" and
     "error message" sidebands instead?

   - Is band #2 meant for human consumption, or do we expect the
     other end to interpret and act on it?  If the former, would it
     make sense to send locale information from the client side and
     ask the server side to produce its output with _("message")?
The interpretation of side-band could be changed by introducing a new
extension, couldn't it?
 - The semantics of packet_flush() is suboptimal, and this
   shortcoming seeps through to the protocol mapped to the
   smart-HTTP transport.

   ...
I don't have an answer to this one. So the reaction is, if it is not
"broken" (in pratice, not in theory), don't touch it. I know I'm
burying my head in the sand..
 - The fetch-pack direction does the common-parent discovery but the
   push-pack direction does not.  This is OK for the normal
   fast-forward push, in which case we will see a known commit on
   the tip of the branch we are pushing into, but makes forced push
   inefficient.
Introducing the ref exchange in push-pack could be done in an
extension too, I think.
 - The existing common-parent discovery done on the fetch-pack side
   enumerates commits contiguously traversing the history to the
   past.  We might want to go exponential or Fibonacci to quickly
   find an ancient common commit and bisect the history from there
   (or it might turn out not to be worth it).
Hm.. i'm wondering if we can already do this with v1 if we have enough
man power.
 - We may want to revamp the builtin/receive-pack.c::report() that
   reports the final result of a push back to the pusher to tell the
   exact object name that sits at the updated tips of refs, not just
   refnames.  It will allow the server side to accept a push of
   commit X to a branch, do some "magic" on X (e.g. rebase it on top
   of the current tip, merge it with the current tip, or let a hook
   to rewrite the commit in any way it deems appropriate) and put
   the resulting commit Y at the tip of the branch.  Without such a
   revamp, it is currently not possible to sensibly allow the server
   side to rewrite what got pushed.
Sounds more coding than changing the protocol, which should be
possible with another extension.
 - If we were to start allowing the receiver side to rewrite pushed
   commits, the updated send-pack protocol must be able to send the
   new objects created by that "magic" back to the pusher.  The
   current protocol does not allow the receive-pack to send packdata
   back to send-pack.
Is it cleaner to just initiate another fetch to get that pack? Maybe
race conditions make it not a good option?
-- 
Duy

Re: [PATCH] protocol upload-pack-v2

From: Martin Fick <hidden>
Date: 2016-06-15 23:04:21

The current protocol has the following problems that limit
us:

 - It is not easy to make it resumable, because we
recompute every time.  This is especially problematic for
the initial fetch aka "clone" as we will be talking about
a large transfer. Redirection to a bundle hosted on CDN
might be something we could do transparently.

 - The protocol extension has a fairly low length limit.

 - Because the protocol exchange starts by the server side
advertising all its refs, even when the fetcher is
interested in a single ref, the initial overhead is
nontrivial, especially when you are doing a small
incremental update.  The worst case is an auto-builder
that polls every five minutes, even when there is no new
commits to be fetched.
A lot of focus about the problems with ref advertisement is 
about the obvious problem mentioned above (a bad problem 
indeed).  I would like to add that there is another related 
problem that all potential solutions to the above problem do 
not neccessarily improve.   When polling regularly there is 
also no current efficient way to check on the current state of 
all refs.  It would be nice to also be able to get an 
incremental update on large refs spaces.

Thanks,

-Martin

-- 
The Qualcomm Innovation Center, Inc. is a member of Code 
Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] protocol upload-pack-v2

From: Stefan Beller <hidden>
Date: 2016-06-15 23:04:21

On Thu, Apr 2, 2015 at 3:18 PM, Martin Fick [off-list ref] wrote:
quoted
The current protocol has the following problems that limit
us:

 - It is not easy to make it resumable, because we
recompute every time.  This is especially problematic for
the initial fetch aka "clone" as we will be talking about
a large transfer. Redirection to a bundle hosted on CDN
might be something we could do transparently.

 - The protocol extension has a fairly low length limit.

 - Because the protocol exchange starts by the server side
advertising all its refs, even when the fetcher is
interested in a single ref, the initial overhead is
nontrivial, especially when you are doing a small
incremental update.  The worst case is an auto-builder
that polls every five minutes, even when there is no new
commits to be fetched.
A lot of focus about the problems with ref advertisement is
about the obvious problem mentioned above (a bad problem
indeed).  I would like to add that there is another related
problem that all potential solutions to the above problem do
not neccessarily improve.   When polling regularly there is
also no current efficient way to check on the current state of
all refs.  It would be nice to also be able to get an
incremental update on large refs spaces.
I think once the new protocol is in place, the server could advertise
the capability to send a differential of refs.

To make sure that works the capability phase should be strictly separated
from the rest, so you can think of any new fancy scheme to transmit
refs or objects, and once both client and server agree on that fancy scheme
both know when to expect the "new changed" protocol.

So from a high level perspective it should look like:
Phase 1) negotiation of capabilities
Phase 2) ref advertisement (i.e. changes in the DAG end points)
Phase 3) transmitting the missing blobs.

The crucial point now is to make sure Phase 1) is not growing to large in
transmission size / required compute power (/ complexity).

And as everybody out there wants to invent new schemes how to do 2) and 3)
efficient, I wonder if we need to do Phase 1) as a differential as well, so I'd
presume the optimum could look like

Client: Last time we talked the capabilities you advertised hashed to $SHA
Server: That's right, but additionally I have "push_cert_nonce=$value"

In the non-optimal case:
Client: Last time we talked the capabilities you advertised hashed to $SHA
Server: I don't know that value, here comes the list of all
capabilities I can do:
 ...
 ...

I like that approach as it would really break down to transmitting the minimal
amount of information most of the time. The downside is to know which
capabilities are cache-able and then hash-able, such that the remote side
only needs to maintain only a very small set of advertised capability lists
and their hash. For example the nonce for signed pushes will hopefully
never be the same, so it makes no sense to have them inside the capabilities
cache.

Having such a capabilities cache would give us a long time until the
phase to negotiate the capabilities will grow too large again (most of the
capabilities I'd assume are rather static per server)

And the way I understand the current situation, it's all about talking this
early negotiation phase, which then allows us to model the refs
advertisement and
the blob transmission later on as a response to upcoming problems in the future.
Thanks,

-Martin

--
The Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] protocol upload-pack-v2

From: Stefan Beller <hidden>
Date: 2016-06-15 23:04:21

After looking at $gmane/264000 again, maybe the client should talk first
stating all the relevant information it wants to get, though I realize this
is not part of capabilities so maybe it could even before, such as:

Client: All I want to do is an ls-remote, so only Phase 2, no
transmission of blobs phase 3
Server: ok
Client[as in the previous mail]: Last time we talked you advertised
hashed to $SHA
Server: that's correct!

As the server knows the client doesn't want to know about Phase3, it
can omit things
relevant to that phase such as the signed push nonce.

So from a high level perspective we'd maybe need 4 phases like
Phase 0) declare the intent (fetch/push all or partial parts)
Phase 1) negotiation of capabilities
Phase 2) ref advertisement (i.e. changes in the DAG end points)
Phase 3) transmitting the missing blobs.

The problem may be that phase 0 and 1 may require mixing, as you may want
to declare new things to do in 0) which you would have needed to advertise as
a capability in 1). So maybe we need to swap that around:

Phase 1a) negotiation of capabilities
Phase 1b) negotiation of intent (fetch/push of all/few branches in
full/shallow/narrow fashion)
Phase 2) ref advertisement (i.e. changes in the DAG end points)
Phase 3) transmitting the missing blobs.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help