git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Ben Peart <Ben.Peart@microsoft.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	Mike Hommey <mh@glandium.org>,
	Lars Schneider <larsxschneider@gmail.com>,
	Eric Wong <e@80x24.org>,
	Christian Couder <chriscool@tuxfamily.org>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Beat Bolli <dev+git@drbeat.li>
Subject: Re: [PATCH v3 03/11] promisor-remote: implement promisor_remote_get_direct()
Date: Mon, 1 Apr 2019 18:41:44 +0200	[thread overview]
Message-ID: <CAP8UFD1SK2mFBYJLqY9VE_Z5tQ5Mor2guc-X3yTbzfUZbHM_2Q@mail.gmail.com> (raw)
In-Reply-To: <xmqqpnqve71d.fsf@gitster-ct.c.googlers.com>

On Wed, Mar 13, 2019 at 5:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:

> > +int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
> > +{
> > +     struct promisor_remote *o;
> > +
> > +     promisor_remote_init();
> > +
> > +     for (o = promisors; o; o = o->next) {
> > +             if (fetch_objects(o->remote_name, oids, oid_nr) < 0)
> > +                     continue;
> > +             return 0;
>
> Suppose the caller asks to fetch 3 objects, A, B and C, from two
> promisors.  The first promisor can give you A and B but cannot give
> you C.  The second promisor only can give you C.
>
> Does fetch_objects() return failure after attempting to fetch A, B
> and C from the first promisor?

Yes, I think it should still send the objects it has and then fail.

Maybe it doesn't work like this right now (I haven't checked), or
maybe the failure should be different than a regular failure. In those
cases I can maybe improve on that in a later iteration.

> Then we go on to the second promisor
> but do we ask all three?  That would mean the second promisor will
> also fail because it cannot give you A and B, and then the whole
> thing would fail.  It would be nicer if the mechanism would allow us
> to fetch what is still missing from later promisor, perhaps.

Yeah, I implemented fetching only what is still missing in v4.

> As the original "fetch" protocol only allows you to fetch a pack
> with everything you asked for in it, instead of feeding you a pack
> with best effort, I think the answer to the above is "it is very
> hard to improve over what we have here", but people may have
> interesting ideas ;-)

I think we should make it best effort when acting as a promisor
remote. I will take a look at that.

> > +     }
> > +
> > +     return -1;
> > +}
> > +
>
> Adding trailing blank line at the end?

Removed.

  reply	other threads:[~2019-04-01 16:41 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-12 13:29 [PATCH v3 00/11] Many promisor remotes Christian Couder
2019-03-12 13:29 ` [PATCH v3 01/11] fetch-object: make functions return an error code Christian Couder
2019-03-12 13:29 ` [PATCH v3 02/11] Add initial support for many promisor remotes Christian Couder
2019-03-13  4:09   ` Junio C Hamano
2019-03-13  4:34     ` Junio C Hamano
2019-04-01 16:41     ` Christian Couder
2019-03-12 13:29 ` [PATCH v3 03/11] promisor-remote: implement promisor_remote_get_direct() Christian Couder
2019-03-13  4:23   ` Junio C Hamano
2019-04-01 16:41     ` Christian Couder [this message]
2019-03-12 13:29 ` [PATCH v3 04/11] promisor-remote: add promisor_remote_reinit() Christian Couder
2019-03-13  4:28   ` Junio C Hamano
2019-04-01 16:41     ` Christian Couder
2019-03-12 13:29 ` [PATCH v3 05/11] promisor-remote: use repository_format_partial_clone Christian Couder
2019-03-13  4:31   ` Junio C Hamano
2019-04-01 16:42     ` Christian Couder
2019-04-01 17:25       ` Junio C Hamano
2019-03-12 13:29 ` [PATCH v3 06/11] Use promisor_remote_get_direct() and has_promisor_remote() Christian Couder
2019-03-12 13:29 ` [PATCH v3 07/11] promisor-remote: parse remote.*.partialclonefilter Christian Couder
2019-03-12 13:29 ` [PATCH v3 08/11] builtin/fetch: remove unique promisor remote limitation Christian Couder
2019-03-12 13:29 ` [PATCH v3 09/11] t0410: test fetching from many promisor remotes Christian Couder
2019-03-12 13:29 ` [PATCH v3 10/11] partial-clone: add multiple remotes in the doc Christian Couder
2019-03-12 13:29 ` [PATCH v3 11/11] remote: add promisor and partial clone config to " Christian Couder

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAP8UFD1SK2mFBYJLqY9VE_Z5tQ5Mor2guc-X3yTbzfUZbHM_2Q@mail.gmail.com \
    --to=christian.couder@gmail.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=chriscool@tuxfamily.org \
    --cc=dev+git@drbeat.li \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=larsxschneider@gmail.com \
    --cc=mh@glandium.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --subject='Re: [PATCH v3 03/11] promisor-remote: implement promisor_remote_get_direct()' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).