Re: [PATCH 1/2] http: use unique tempfiles for packfile URI downloads

[Junio C Hamano <gitster@pobox.com>]

Ted Nyman <tnyman@openai.com> writes:

> Since 8d5d2a34df (http-fetch: support fetching packfiles by URL,
> 2020-06-10), packfile URI downloads have been staged at
> objects/pack/pack-<hash>.pack.temp.
>
> The path is derived from the advertised pack hash. Two processes
> fetching the same pack into a shared object database therefore open the
> same file for append. Their writes can corrupt the temporary pack. If
> one process arrives after the other has completed the download, it may
> instead try to resume at EOF, which some HTTP servers reject with 416.
>
> Use the tempfile API to give direct packfile URI downloads unique
> temporary files. Keep the deterministic path for ordinary dumb HTTP
> pack requests, which use it to resume a partial download left by an
> earlier invocation.
>
> This means that a packfile URI download cannot be resumed by a later
> invocation. A retry starts with an empty temporary file instead.

While that does sound like a safe and correct approach, stepping
back briefly, would it not be wasteful for the second process to
download the same packfile that the first has already started
downloading?

Are there better ways for these processes to coordinate with each
other?  Instead of appending to the file, what if the second process
uses a predictable temporary name (which we already use) to open a
new file with O_CREAT | O_EXCL to avoid this redundant work?  If the
open call fails because the file already exists, the second process
can detect that another process is active and wait for it to finish
rather than initiating its own network request.

Doing so might require setting up a trigger or polling mechanism to
wait for the first process's download to complete (and detecting if
the other process dies without cleaning up), though that may open a
can of worms.