git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Jeff Hostetler <git@jeffhostetler.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Jeff King <peff@peff.net>, Ben Peart <Ben.Peart@microsoft.com>,
	Jonathan Tan <jonathantanmy@google.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>
Subject: Re: [PATCH 10/40] external-odb: implement external_odb_get_direct
Date: Mon, 15 Jan 2018 15:47:34 +0100	[thread overview]
Message-ID: <CAP8UFD2MR_VCgFKVmLEaO989uM_AvX8_134PdoT=4MyJVPD1nA@mail.gmail.com> (raw)
In-Reply-To: <4ce872c0-8f4b-602a-9ee3-cf429cc6a146@jeffhostetler.com>

On Thu, Jan 4, 2018 at 6:44 PM, Jeff Hostetler <git@jeffhostetler.com> wrote:
>
>
> On 1/3/2018 11:33 AM, Christian Couder wrote:
>>
>> This is implemented only in the promisor remote mode
>> for now by calling fetch_object().
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>   external-odb.c | 15 +++++++++++++++
>>   external-odb.h |  1 +
>>   odb-helper.c   | 13 +++++++++++++
>>   odb-helper.h   |  3 ++-
>>   4 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/external-odb.c b/external-odb.c
>> index d26e63d8b1..5d0afb9762 100644
>> --- a/external-odb.c
>> +++ b/external-odb.c
>> @@ -76,3 +76,18 @@ int external_odb_has_object(const unsigned char *sha1)
>>                         return 1;
>>         return 0;
>>   }
>> +
>> +int external_odb_get_direct(const unsigned char *sha1)
>> +{
>> +       struct odb_helper *o;
>> +
>> +       external_odb_init();
>> +
>> +       for (o = helpers; o; o = o->next) {
>> +               if (odb_helper_get_direct(o, sha1) < 0)
>> +                       continue;
>> +               return 0;
>
>> +     }
>
> Would this be simpler said as:
>         for (o = ...)
>                 if (!odb_helper_get_direct(...))
>                         return 0;

At then end of the series the content of the loop is:

        if (!(o->supported_capabilities & ODB_HELPER_CAP_GET_DIRECT))
            continue;
        if (odb_helper_get_direct(o, sha1) < 0)
            continue;
        return 0;

And I think it is fine like that, so I don't think changing this
commit is a good idea.

>> +       return -1;
>> +}
>> diff --git a/external-odb.h b/external-odb.h
>> index 9a3c2f01b3..fd6708163e 100644
>> --- a/external-odb.h
>> +++ b/external-odb.h
>> @@ -4,5 +4,6 @@
>>   extern int has_external_odb(void);
>>   extern const char *external_odb_root(void);
>>   extern int external_odb_has_object(const unsigned char *sha1);
>> +extern int external_odb_get_direct(const unsigned char *sha1);
>>     #endif /* EXTERNAL_ODB_H */
>> diff --git a/odb-helper.c b/odb-helper.c
>> index 1404393807..4b70b287af 100644
>> --- a/odb-helper.c
>> +++ b/odb-helper.c
>> @@ -4,6 +4,7 @@
>>   #include "odb-helper.h"
>>   #include "run-command.h"
>>   #include "sha1-lookup.h"
>> +#include "fetch-object.h"
>>     struct odb_helper *odb_helper_new(const char *name, int namelen)
>>   {
>> @@ -52,3 +53,15 @@ int odb_helper_has_object(struct odb_helper *o, const
>> unsigned char *sha1)
>>         return !!odb_helper_lookup(o, sha1);
>>   }
>>   +int odb_helper_get_direct(struct odb_helper *o,
>> +                         const unsigned char *sha1)
>> +{
>> +       int res = 0;
>> +       uint64_t start = getnanotime();
>> +
>> +       fetch_object(o->dealer, sha1);
>> +
>> +       trace_performance_since(start, "odb_helper_get_direct");
>> +
>> +       return res;
>
>
> 'res' will always be 0, so the external_odb_get_direct() will
> only do the first helper.  i haven't looked at the rest of the
> series yet, so maybe you've already addressed this.

That's why I previously suggested in one of your or Jonathan's patch
that fetch_object() should return an int that tells the caller if the
object has been fetched instead of void.

If we make it possible at one point to have the objects fetched
fetch_object() in different remotes (and I think that's a
straightforward goal), this will actually fail but callers will have
no simple way to know that.

> Also, I put a TODO comment in the fetch_object() header to
> consider returning an error/success, so maybe that could help
> here too.

Yeah, indeed.

  reply	other threads:[~2018-01-15 14:47 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-03 16:33 [PATCH 00/40] Promisor remotes and external ODB support Christian Couder
2018-01-03 16:33 ` [PATCH 01/40] Add initial external odb support Christian Couder
2018-01-04 19:59   ` Jeff Hostetler
2018-01-15 14:34     ` Christian Couder
2018-01-03 16:33 ` [PATCH 02/40] Add GIT_NO_EXTERNAL_ODB env variable Christian Couder
2018-01-03 16:33 ` [PATCH 03/40] external-odb: add has_external_odb() Christian Couder
2018-01-03 16:33 ` [PATCH 04/40] fsck: introduce promisor objects Christian Couder
2018-01-03 16:33 ` [PATCH 05/40] fsck: support refs pointing to " Christian Couder
2018-01-03 16:33 ` [PATCH 06/40] fsck: support referenced " Christian Couder
2018-01-03 16:33 ` [PATCH 07/40] fsck: support promisor objects as CLI argument Christian Couder
2018-01-03 16:33 ` [PATCH 08/40] index-pack: refactor writing of .keep files Christian Couder
2018-01-03 16:33 ` [PATCH 09/40] introduce fetch-object: fetch one promisor object Christian Couder
2018-01-03 16:33 ` [PATCH 10/40] external-odb: implement external_odb_get_direct Christian Couder
2018-01-04 17:44   ` Jeff Hostetler
2018-01-15 14:47     ` Christian Couder [this message]
2018-01-03 16:33 ` [PATCH 11/40] sha1_file: support lazily fetching missing objects Christian Couder
2018-01-03 16:33 ` [PATCH 12/40] rev-list: support termination at promisor objects Christian Couder
2018-01-03 16:33 ` [PATCH 13/40] gc: do not repack promisor packfiles Christian Couder
2018-01-03 16:33 ` [PATCH 14/40] sha1_file: prepare for external odbs Christian Couder
2018-01-04 18:00   ` Jeff Hostetler
2018-01-16  7:23     ` Christian Couder
2018-01-03 16:33 ` [PATCH 15/40] external-odb: add script mode support Christian Couder
2018-01-04 19:55   ` Jeff Hostetler
2018-03-19 13:15     ` Christian Couder
2018-01-03 16:33 ` [PATCH 16/40] odb-helper: add 'enum odb_helper_type' Christian Couder
2018-01-03 16:33 ` [PATCH 17/40] odb-helper: add odb_helper_init() to send 'init' instruction Christian Couder
2018-01-03 16:33 ` [PATCH 18/40] t0400: add 'put_raw_obj' instruction to odb-helper script Christian Couder
2018-01-03 16:33 ` [PATCH 19/40] external odb: add 'put_raw_obj' support Christian Couder
2018-01-03 16:33 ` [PATCH 20/40] external-odb: accept only blobs for now Christian Couder
2018-01-03 16:33 ` [PATCH 21/40] t0400: add test for external odb write support Christian Couder
2018-01-03 16:33 ` [PATCH 22/40] Add t0410 to test external ODB transfer Christian Couder
2018-01-03 16:33 ` [PATCH 23/40] lib-httpd: pass config file to start_httpd() Christian Couder
2018-01-03 16:33 ` [PATCH 24/40] lib-httpd: add upload.sh Christian Couder
2018-01-03 16:33 ` [PATCH 25/40] lib-httpd: add list.sh Christian Couder
2018-01-03 16:33 ` [PATCH 26/40] lib-httpd: add apache-e-odb.conf Christian Couder
2018-01-03 16:33 ` [PATCH 27/40] odb-helper: add odb_helper_get_raw_object() Christian Couder
2018-01-03 16:33 ` [PATCH 28/40] pack-objects: don't pack objects in external odbs Christian Couder
2018-01-04 20:54   ` Jeff Hostetler
2018-03-19 13:27     ` Christian Couder
2018-01-03 16:33 ` [PATCH 29/40] Add t0420 to test transfer to HTTP external odb Christian Couder
2018-01-03 16:33 ` [PATCH 30/40] external-odb: add 'get_direct' support Christian Couder
2018-01-03 16:33 ` [PATCH 31/40] odb-helper: add 'script_mode' to 'struct odb_helper' Christian Couder
2018-01-03 16:33 ` [PATCH 32/40] odb-helper: add init_object_process() Christian Couder
2018-01-03 16:33 ` [PATCH 33/40] Add t0450 to test 'get_direct' mechanism Christian Couder
2018-01-03 16:33 ` [PATCH 34/40] Add t0460 to test passing git objects Christian Couder
2018-01-03 16:33 ` [PATCH 35/40] odb-helper: add put_object_process() Christian Couder
2018-01-03 16:33 ` [PATCH 36/40] Add t0470 to test passing raw objects Christian Couder
2018-01-03 16:34 ` [PATCH 37/40] odb-helper: add have_object_process() Christian Couder
2018-01-03 16:34 ` [PATCH 38/40] Add t0480 to test "have" capability and raw objects Christian Couder
2018-01-03 16:34 ` [PATCH 39/40] external-odb: use 'odb=magic' attribute to mark odb blobs Christian Couder
2018-01-03 16:34 ` [PATCH 40/40] Add Documentation/technical/external-odb.txt 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='CAP8UFD2MR_VCgFKVmLEaO989uM_AvX8_134PdoT=4MyJVPD1nA@mail.gmail.com' \
    --to=christian.couder@gmail.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=chriscool@tuxfamily.org \
    --cc=e@80x24.org \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=larsxschneider@gmail.com \
    --cc=mh@glandium.org \
    --cc=pclouds@gmail.com \
    --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.
Code repositories for project(s) associated with this public 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).