From: Jeff King <firstname.lastname@example.org> To: "René Scharfe" <email@example.com> Cc: "Junio C Hamano" <firstname.lastname@example.org>, 唐宇奕 <email@example.com>, firstname.lastname@example.org Subject: Re: Bug report: orphaned pack-objects after killing upload-pack on [ Date: Tue, 1 Dec 2020 07:15:13 -0500 [thread overview] Message-ID: <X8Yz0bY1LOHpKxKA@coredump.intra.peff.net> (raw) In-Reply-To: <email@example.com> On Thu, Nov 26, 2020 at 09:04:35PM +0100, René Scharfe wrote: > >> We spawn an external pack-objects process to actually send objects to > >> the remote side. If we are killed by a signal during this process, > >> then pack-objects may continue to run. As soon as it starts producing > >> output for the pack, it will see a failure writing to upload-pack and > >> exit itself. But before then, it may do significant work traversing > >> the object graph, compressing deltas, etc, which will all be > >> pointless. So let's make sure to kill as soon as we know that the > >> caller will not read the result. > > > > Thanks, that reads well. > > > The patch is trivial, you don't need my sign-off. You could record Peff > as its author, as he contributed the most to the version in seen. I didn't want this topic to be forgotten, so here it is with me as the author, my signoff, and an overview of the reproduction in the commit message. (I am perfectly happy for René to be author, but I am mainly interested in resolving the signoff issue; I agree most of the work was in the diagnosis, and I did re-type the single line all by myself ;) ). -- >8 -- Subject: [PATCH] upload-pack: kill pack-objects helper on signal or exit We spawn an external pack-objects process to actually send objects to the remote side. If we are killed by a signal during this process, then pack-objects may continue to run. As soon as it starts producing output for the pack, it will see a failure writing to upload-pack and exit itself. But before then, it may do significant work traversing the object graph, compressing deltas, etc, which will all be pointless. So let's make sure to kill as soon as we know that the caller will not read the result. There's no test here, since it's inherently racy, but here's an easy reproduction is on a large-ish repo like linux.git: - make sure you don't have pack bitmaps (since they make the enumerating phase go quickly). For linux.git it takes ~30s or so to walk the whole graph on my machine. - run "git clone --no-local -q . dst"; the "-q" is important because if pack-objects is writing progress to upload-pack (to get multiplexed over the sideband to the client), then it will notice pretty quickly the failure to write to stderr - kill the client-side clone process in another terminal (don't use ^C, as that will send SIGINT to all of the processes) - run "ps au | grep git" or similar to observe upload-pack dying within 5 seconds (it will send a keepalive that will notice the client has gone away) - but you'll still see pack-objects consuming 100% CPU (and 1GB+ of RAM) during the traversal and delta compression phases. It will exit as soon as it starts to write the pack (when it will notice that upload-pack went away). With this patch, pack-objects exits as soon as upload-pack does. Signed-off-by: Jeff King <firstname.lastname@example.org> --- upload-pack.c | 1 + 1 file changed, 1 insertion(+) diff --git a/upload-pack.c b/upload-pack.c index 5dc8e1f844..1006bebd50 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -321,6 +321,7 @@ static void create_pack_file(struct upload_pack_data *pack_data, pack_objects.in = -1; pack_objects.out = -1; pack_objects.err = -1; + pack_objects.clean_on_exit = 1; if (start_command(&pack_objects)) die("git upload-pack: unable to fork git-pack-objects"); -- 22.214.171.1243.g57eb4d1d5a
next prev parent reply other threads:[~2020-12-01 12:17 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-19 8:18 唐宇奕 2020-11-20 18:52 ` Bug report: orphaned pack-objects after killing upload-pack on [was: (no subject)] René Scharfe 2020-11-21 0:29 ` Jeff King 2020-11-21 21:54 ` Bug report: orphaned pack-objects after killing upload-pack on [ Junio C Hamano 2020-11-24 3:21 ` 唐宇奕 2020-11-24 9:11 ` Jeff King 2020-11-25 21:42 ` Junio C Hamano 2020-11-26 0:53 ` Jeff King 2020-11-26 1:04 ` Junio C Hamano 2020-11-26 20:04 ` René Scharfe 2020-11-27 4:17 ` Jeff King 2020-11-27 20:43 ` René Scharfe 2020-11-28 6:30 ` Jeff King 2020-12-01 12:15 ` Jeff King [this message] 2020-12-02 11:45 ` René Scharfe 2020-12-02 22:14 ` Junio C Hamano
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=X8Yz0bY1LOHpKxKA@coredump.intra.peff.net \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: Bug report: orphaned pack-objects after killing upload-pack on [' \ /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).