UploadPack v2 protocol: Stop negotiation for orphan refs

The fetch of a single orphan ref (for example Gerrit meta ref:
refs/changes/21/21/meta) did not stop the negotiation so client
had to advertise all refs. This impacts the fetch performance
on repositories with a large number of refs (for example on
Gerrit repository it takes 20 seconds to fetch meta ref
comparing to 1.2 second to fetch ref with parent).

To avoid this issue UploadPack, used on the server side,
now checks if all `want` refs have parents, if not this
means that client doesn't need any extra objects, hence
the server responds with `ready` and finishes the
negotiation phase.

Bug: 577937
Change-Id: Ia3001b400b415d5cf6aae45e72345ca08d3af058
This commit is contained in:
Marcin Czech 2021-12-22 17:42:36 +01:00
parent 8a4b983767
commit 78d4fb1ca0
2 changed files with 60 additions and 0 deletions

View File

@ -998,6 +998,61 @@ public void testV2FetchClientStopsNegotiation() throws Exception {
assertTrue(client.getObjectDatabase().has(barChild.toObjectId()));
}
@Test
public void testV2FetchServerStopsNegotiationForRefWithoutParents()
throws Exception {
RevCommit fooCommit = remote.commit().message("x").create();
RevCommit barCommit = remote.commit().message("y").create();
remote.update("refs/changes/01/1/1", fooCommit);
remote.update("refs/changes/02/2/1", barCommit);
ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n",
PacketLineIn.delimiter(),
"want " + fooCommit.toObjectId().getName() + "\n",
"have " + barCommit.toObjectId().getName() + "\n",
PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("acknowledgments"));
assertThat(pckIn.readString(),
is("ACK " + barCommit.toObjectId().getName()));
assertThat(pckIn.readString(), is("ready"));
assertTrue(PacketLineIn.isDelimiter(pckIn.readString()));
assertThat(pckIn.readString(), is("packfile"));
parsePack(recvStream);
assertTrue(client.getObjectDatabase().has(fooCommit.toObjectId()));
}
@Test
public void testV2FetchServerDoesNotStopNegotiationWhenOneRefWithoutParentAndOtherWithParents()
throws Exception {
RevCommit fooCommit = remote.commit().message("x").create();
RevCommit barParent = remote.commit().message("y").create();
RevCommit barChild = remote.commit().message("y").parent(barParent)
.create();
RevCommit fooBarParent = remote.commit().message("z").create();
RevCommit fooBarChild = remote.commit().message("y")
.parent(fooBarParent)
.create();
remote.update("refs/changes/01/1/1", fooCommit);
remote.update("refs/changes/02/2/1", barChild);
remote.update("refs/changes/03/3/1", fooBarChild);
ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n",
PacketLineIn.delimiter(),
"want " + fooCommit.toObjectId().getName() + "\n",
"want " + barChild.toObjectId().getName() + "\n",
"want " + fooBarChild.toObjectId().getName() + "\n",
"have " + fooBarParent.toObjectId().getName() + "\n",
PacketLineIn.end());
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("acknowledgments"));
assertThat(pckIn.readString(),
is("ACK " + fooBarParent.toObjectId().getName()));
assertTrue(PacketLineIn.isEnd(pckIn.readString()));
}
@Test
public void testV2FetchThinPack() throws Exception {
String commonInBlob = "abcdefghijklmnopqrstuvwxyz";

View File

@ -2103,6 +2103,11 @@ private boolean wantSatisfied(RevObject want) throws IOException {
if (want.has(SATISFIED))
return true;
if (((RevCommit) want).getParentCount() == 0) {
want.add(SATISFIED);
return true;
}
walk.resetRetain(SAVE);
walk.markStart((RevCommit) want);
if (oldestTime != 0)