All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
Date: Thu, 26 Jan 2017 16:26:47 -0800	[thread overview]
Message-ID: <67afbb3b-5d0b-8c0d-3f6e-3f559c68f4bd@google.com> (raw)
In-Reply-To: <20170126230046.aknesybfyzxhx3ia@sigill.intra.peff.net>

Thanks for your comments.

On 01/26/2017 03:00 PM, Jeff King wrote:
> On Wed, Jan 25, 2017 at 02:02:53PM -0800, Jonathan Tan wrote:
>
>> Negotiation currently happens by upload-pack initially sending a list of
>> refs with names and SHA-1 hashes, and then several request/response
>> pairs in which the request from fetch-pack consists of SHA-1 hashes
>> (selected from the initial list). Allowing the request to consist of
>> names instead of SHA-1 hashes increases tolerance to refs changing
>> (due to time, and due to having load-balanced servers without strong
>> consistency),
>
> Interesting. My big question is: what happens when a ref _does_ change?
> How does the client handle this?
>
> The existing uploadpack.allowReachableSHA1InWant is there to work around
> the problem that an http client may get a ref advertisement in one step,
> and then come back later to do the want/have negotiation, at which point
> the server has moved on (or maybe it's even a different server). There
> the client says "I want sha1 X", and the server needs to say "well, X
> isn't my tip now, but it's still acceptable for you to fetch".
>
> But this seems to go in the opposite direction. After the advertisement,
> the client decides "OK, I want to fetch refs/heads/master which is at
> SHA1 X". It connects to the server and says "I want refs/heads/master".
> Let's say the server has moved its version of the ref to SHA1 Y.
>
> What happens? I think the server will say "wanted-ref master Y". Does
> the client just decide to use "Y" then?  How does that interact with any
> decisions the client might have made about X? I guess things like
> fast-forwards have to be decided after we fetch the objects anyway
> (since we cannot compute them until we get the pack), so maybe there
> aren't any such decisions. I haven't checked.

Yes, the server will say "wanted-ref master Y". The relevant code 
regarding the decisions the client makes regarding X or Y is in do_fetch 
in builtin/fetch.c.

There, I can see these decisions done using X:
  - check_not_current_branch (forbidding fetching that modifies the
    current branch) (I just noticed that this has to be done for Y too,
    and will do so)
  - prune refs [*]
  - automatic tag following [*]

[*] X and Y may differ in that one relevant ref appears in one set but 
not in the other (because a ref was added or removed in the meantime), 
causing a different result if these decisions were to be done using Y, 
but I think that it is OK either way.

Fetch optimizations (for example, everything_local in fetch-pack.c) that 
check if the client really needs to fetch are also done using X, of 
course (and if the optimization succeeds, there is no Y).

Fast-forwards (and everything else in store_updated_refs) are decided 
using Y.

>> and is a step towards eliminating the need for the server
>> to send the list of refs first (possibly improving performance).
>
> I'm not sure it is all that useful towards that end. You still have to
> break compatibility so that the client tells the server to suppress the
> ref advertisement. After that, it is just a question of asking for the
> refs. And you have two options:
>
>   1. Ask the server to tell you the values of some subset of the refs,
>      pick what you want, and then do the want/have as normal.
>
>   2. Go straight to the want/have, but tell the server the refs you want
>      instead of their sha1s.
>
> I think your approach here would lead to (2).
>
> But (1), besides being closer to how the protocol works now, seems like
> it's more flexible. I can ask about the ref state without necessarily
> having to retrieve the objects. How would you write git-ls-remote with
> such a system?

Assuming a new protocol with the appropriate backwards compatibility 
(which would have to be done for both options), (2) does save a 
request/response pair (because we can send the ref names directly as 
"want-ref" in the initial request instead of sending ref names in the 
initial request and then confirming them using "want <SHA-1>" in the 
subsequent request). Also, (2) is more tolerant towards refs changing 
over time. (1) might be closer to the current protocol, but I think that 
the difference is not so significant (only in "want-ref" vs "want" 
request and the "wanted-ref" response).

As for git-ls-remote, I currently think that it would have to use the 
existing protocol.

>> [1] There has been some discussion about whether the server should
>> accept partial ref names, e.g. [2]. In this patch set, I have made the
>> server only accept full names, and it is the responsibility of the
>> client to send the multiple patterns which it wants to match. Quoting
>> from the commit message of the second patch:
>>
>>   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.
>
> That has a cost that scales linearly with the number of refs, because
> you have to ask for each name 6 times.  After the discussion you linked,
> I think my preference is more like:
>
>   1. Teach servers to accept a list of patterns from the client
>      which will be resolved in order. Unlike your system, the client
>      only needs to specify the list once per session, rather than once
>      per ref.
>
>   2. (Optional) Give a shorthand for the stock patterns that git has had
>      in place for years. That saves some bytes over specifying the
>      patterns completely (though it's really not _that_ many bytes, so
>      perhaps the complication isn't a big deal).

I envision that most fetches would be done on a single name (with or 
without globs), that expands to 5 (so it is not so crucial to factor out 
the list of patterns). Having said that, I am open to changing this.

  reply	other threads:[~2017-01-27  0:27 UTC|newest]

Thread overview: 31+ 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
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 [this message]
2017-02-07 23:53 ` Jonathan Tan
2017-02-09  0:26   ` Junio C Hamano
2018-05-20  8:24 Apinan Ponchan

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=67afbb3b-5d0b-8c0d-3f6e-3f559c68f4bd@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.