All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC 02/14] upload-pack: allow ref name and glob requests
Date: Thu, 26 Jan 2017 14:23:36 -0800	[thread overview]
Message-ID: <xmqqd1f931g7.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com> (Jonathan Tan's message of "Wed, 25 Jan 2017 14:02:55 -0800")

Jonathan Tan <jonathantanmy@google.com> writes:

> Currently, while performing packfile negotiation [1], upload-pack allows
> clients to specify their desired objects only as SHA-1s. This causes:
> (a) vulnerability to failure when an object turns non-existent during
>     negotiation, which may happen if, for example, upload-pack is
>     provided by multiple Git servers in a load-balancing arrangement,
>     and
> (b) dependence on the server first publishing a list of refs with
>     associated objects.
>
> To eliminate (a) and take a step towards eliminating (b), teach
> upload-pack to support requests in the form of ref names and globs (in
> addition to the existing support for SHA-1s) through a new line of the
> form "want-ref <ref>" where ref is the full name of a ref, a glob
> pattern, or a SHA-1. At the conclusion of negotiation, the server will
> write "wanted-ref <SHA-1> <name>" for all requests that have been
> specified this way.

I am not sure if this "at the conclusion of" is sensible.  It is OK
to assume that what the client side has is fixed, and it is probably
OK to desire that what the server side has can change, but at the
same time, it feels quite fragile to move the goalpost in between.

Stepping back a bit, in an environment that involves multiple server
instances that have inconsistent set of refs, can the negotiation
even be sensibly and safely implemented?  The first server the
client contacts may, in response to a "have", say "I do have that
commit so you do not have to send its ancestors to me.  We found one
cut-off point.  Please do explore other lines of histories."  The
next server that concludes the negotiation exchange may not have
that commit and will be unable to produce a pack that excludes the
objects reachable from that commit---wouldn't that become a problem?

One way to prevent such a problem from hurting clients may be for
these multiple server instances to coordinate and make sure they
have a shared perception of the common history among them.  Some
pushes may have come to one instance but may not have propagated to
other instances, and such a commit cannot be accepted as usable
"have" if the servers anticipate that the final client request would
go to any of the servers.  Otherwise the multiple server arrangement
would not work safely, methinks.

And if the servers are ensuring the safety using such a mechanism,
they can use the same mechanism to restrain "faster" instances from
sending too fresh state of refs that other instances haven't caught
up to, which would mean they can present a consistent set of refs to
the client in the first place, no?

So I am not sure if the mechanism to request history by refname
instead of the tip commit would help the multi-server environment as
advertised.  It may help solving other problems, though (e.g. like
"somebody pushed to update after the initial advertisement was sent
out" which can happen even in a single server environment).

> The server indicates that it supports this feature by advertising the
> capability "ref-in-want". Advertisement of this capability is by default
> disabled, but can be enabled through a configuration option.

OK.

> To be flexible with respect to client needs, the server does not
> indicate an error if a "want-ref" line corresponds to no refs, but
> instead relies on the client to ensure that what the user needs has been
> fetched. For example, a client could reasonably expand an abbreviated
> name "foo" to "want-ref foo", "want-ref refs/heads/foo", "want-ref
> refs/tags/foo", among others, and ensure that at least one such ref has
> been fetched.

Cute.  This may be one way to implement the DWIM thing within the
constraint of eventually wanting to go to "client speaks first, the
server does not advertise things the client is not interested in"
world.  

But at the same time it may end up bloating the set of refs the
client asks instead.  Instead of receiving the advertisement and
then sending one request after picking the matching one from it,
the client needs to send "refs/{heads,tags,whatever}/foo".

  reply	other threads:[~2017-01-26 22:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-25 22:02 [RFC 00/14] Allow fetch-pack to send ref names (globs allowed) Jonathan Tan
2017-01-25 22:02 ` [RFC 01/14] upload-pack: move parsing of "want" line Jonathan Tan
2017-01-25 22:02 ` [RFC 02/14] upload-pack: allow ref name and glob requests Jonathan Tan
2017-01-26 22:23   ` Junio C Hamano [this message]
2017-01-27  0:35     ` Jonathan Tan
2017-01-27  1:54       ` Junio C Hamano
2017-01-25 22:02 ` [RFC 03/14] upload-pack: test negotiation with changing repo Jonathan Tan
2017-01-26 22:33   ` Junio C Hamano
2017-01-27  0:44     ` Jonathan Tan
2017-02-22 23:36       ` Junio C Hamano
2017-02-23 18:43         ` [PATCH] upload-pack: report "not our ref" to client Jonathan Tan
2017-02-23 20:14           ` Junio C Hamano
2017-01-25 22:02 ` [RFC 04/14] fetch: refactor the population of hashes Jonathan Tan
2017-01-25 22:02 ` [RFC 05/14] fetch: refactor fetch_refs into two functions Jonathan Tan
2017-01-25 22:02 ` [RFC 06/14] fetch: refactor to make function args narrower Jonathan Tan
2017-01-25 22:03 ` [RFC 07/14] fetch-pack: put shallow info in out param Jonathan Tan
2017-01-25 22:03 ` [RFC 08/14] fetch-pack: check returned refs for matches Jonathan Tan
2017-01-25 22:03 ` [RFC 09/14] transport: put ref oid in out param Jonathan Tan
2017-01-25 22:03 ` [RFC 10/14] fetch-pack: support partial names and globs Jonathan Tan
2017-01-25 22:03 ` [RFC 11/14] fetch-pack: support want-ref Jonathan Tan
2017-01-25 22:03 ` [RFC 12/14] fetch-pack: do not printf after closing stdout Jonathan Tan
2017-01-26  0:50   ` Stefan Beller
2017-01-26 18:18     ` Jonathan Tan
2017-01-25 22:03 ` [RFC 13/14] fetch: send want-ref and receive fetched refs Jonathan Tan
2017-01-25 22:03 ` [RFC 14/14] DONT USE advertise_ref_in_want=1 Jonathan Tan
2017-01-26 22:15 ` [RFC 00/14] Allow fetch-pack to send ref names (globs allowed) Stefan Beller
2017-01-26 23:00 ` Jeff King
2017-01-27  0:26   ` Jonathan Tan
2017-02-07 23:53 ` Jonathan Tan
2017-02-09  0:26   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqd1f931g7.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.