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
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
index 5045e94..3958de2 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
@@ -999,6 +999,61 @@
}
@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";
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 9fda639..63deff2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -2103,6 +2103,11 @@
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)
