"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: avoid extra tree requests after got-read-pack enumeration
To:
gameoftrees@openbsd.org
Date:
Wed, 15 Jun 2022 00:08:22 +0200

Download raw body.

Thread
On Tue, Jun 14, 2022 at 02:46:24PM +0200, Stefan Sperling wrote:
> Profiling got-read-pack shows a lot of individual tree requests
> being sent by gotadmin pack -a, even if all required commits and
> trees are present in the enumeration pack file.
> This happens because gotadmin will always walk the entire history
> again, the slow way, in order to look for any missing objects.
> 
> To fix this, make got-read-pack indicate whether it managed to locate
> all required objects in its pack file, and only try loading more objects
> if enumeration from the pack file was incomplete.
> 
> Tags need to be loaded in any case because got-read-pack will never
> send tags while enumerating objects. This could still be optimized later.
> 
> ok?

I committed this patch after an off-list ok from op@

Turns out this exposed a bug elsewhere, and 'got send' is currently
broken again as a result. Fix below.

ok?

-----------------------------------------------
commit 949fed073a0669d44791832ce3c26e7f3243ebe3 (fix-send)
from: Stefan Sperling <stsp@stsp.name>
date: Tue Jun 14 22:05:30 2022 UTC
 
 fix a bug in got_privsep_send_object_idlist() exposed by recent changes
 
 The old code did not work correctly if only a single object Id was to
 be sent to got-read-pack. Make got-read-pack error out if the list
 of commits for object enumeration is empty to catch this problem if
 it occurs again.
 
 Found by the send_basic test, which was failing with GOT_TEST_PACK=1
 
diff 9ae64ce0617bf9bfc70ad80119b0a4f3553fc286 5a67f59023bb8af94a46a31b99794f705cbf0e50
blob - f7736221c3c02ffd4e1d422bbe6510131dfc9e74
blob + 708d939b13209d4284366e2f64c33fc1cf476ae5
--- lib/privsep.c
+++ lib/privsep.c
@@ -3119,21 +3119,21 @@ got_privsep_send_object_idlist(struct imsgbuf *ibuf,
 {
 	const struct got_error *err = NULL;
 	struct got_object_id *idlist[GOT_IMSG_OBJ_ID_LIST_MAX_NIDS];
-	int i, j = 0;
+	int i, queued = 0;
 
 	for (i = 0; i < nids; i++) {
-		j = i % nitems(idlist);
-		idlist[j] = ids[i];
-		if (j >= nitems(idlist) - 1) {
-			err = send_idlist(ibuf, idlist, j + 1);
+		idlist[i % nitems(idlist)] = ids[i];
+		queued++;
+		if (queued >= nitems(idlist) - 1) {
+			err = send_idlist(ibuf, idlist, queued);
 			if (err)
 				return err;
-			j = 0;
+			queued = 0;
 		}
 	}
 
-	if (j > 0) {
-		err = send_idlist(ibuf, idlist, j + 1);
+	if (queued > 0) {
+		err = send_idlist(ibuf, idlist, queued);
 		if (err)
 			return err;
 	}
blob - 4b46e10d580fcd5a865ba785169787d0463eb2b0
blob + c852d54f14a6f88d30757457e124d1ba29d63063
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1388,6 +1388,11 @@ enumeration_request(struct imsg *imsg, struct imsgbuf 
 	if (err)
 		goto done;
 
+	if (STAILQ_EMPTY(&commit_ids)) {
+		err = got_error(GOT_ERR_PRIVSEP_MSG);
+		goto done;
+	}
+
 	err = recv_object_ids(idset, ibuf);
 	if (err)
 		goto done;