Re: [RFC PATCH] promisor-remote: always JIT fetch with --refetch

From: Robert Coup <robert.coup@koordinates.com>
Date: Oct 7, 2024, 00:21 UTC
Message-ID: <CAFLLRp+Y1hO6r7mfdghS0q3EyfJhU_e43Hzi9PXgF_EuF9Fuog@mail.gmail.com>
In-Reply-To: <xmqqset8c0o7.fsf@gitster.g>

Hi Emily,

I was the one who originally implemented --refetch in [1][2]

[1] https://lore.kernel.org/git/pull.1138.v4.git.1648476131.gitgitgadget@gmail.com/ [2] https://github.com/gitgitgadget/git/pull/1138

On Sun, 6 Oct 2024 at 23:43, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

>
> Hmph.  The whole lazy fetch business looks more and more broken X-<.
> There is a comment in the refetch code path that tells us to "perform
> a full refetch ignoring existing objects", but if an object truly
> exists, there should be no need to refetch, and it starts to smell
> more like "ignoring somebody who gives us an incorrect information
> that these objects exist".

Basically --refetch was originally designed to send no 'have's during a fetch, the original motivation being changing a partial clone filter and fetching all the newly-applicable trees & blobs in a single transfer.

Show 5 quoted lines

> The documentation for "git fetch --refetch" says that this grabs
> everything as if we are making a fresh clone, ignoring everything we
> already have.  Which makes the change in this patch prohibitively
> expensive for asking each single object lazily from the promisor
> remote, but is that really the case?

From a very quick re-review this is correct that it's expensive: refetch sends no 'have's, so if you pass a single commit oid then it'll fetch all ancestors and all the dependent trees & blobs, duplicating what's in the object store and relying on a repack to clean up. If a commit is missing that's one way to fix it, but it's a pretty nuclear option: feels like your option iv (terminate with an error) leading to fsck invoking/suggesting --refetch might avoid unintentionally recloning the entire repo.

In my original RFC [3], Jonathan Tan suggested that --refetch could be useful to repair missing objects like this, but it was out of scope for me at the time. But maybe there's a way to improve it for this sort of case?

[3] https://lore.kernel.org/git/20220202185957.1928631-1-jonathantanmy@google.com/

Show 7 quoted lines

> Emily Shaffer <emilyshaffer@google.com> writes:
>
> This manifested at $DAYJOB in a repo with the following features:
> * blob-filtered partial clone enabled
> * commit graph enabled
> * ref Foo pointing to commit object 6aaaca
> * object 6aaaca missing[a]

I presume there wasn't an obvious/related cause for commit 6aaaca to go missing in the first place?

Thanks,

Rob :)