Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarcin Czech2021-12-22 16:42:36 +0000
committerMarcin Czech2022-01-18 12:00:03 +0000
commit78d4fb1ca00149a700f586357139f08efaa29ff6 (patch)
tree14e3876a4e8021435c3fbcc98078dcf4e5bb39ec
parent8a4b98376785a6c271fede00035fe89dea2b9982 (diff)
downloadjgit-stable-5.11.tar.gz
jgit-stable-5.11.tar.xz
jgit-stable-5.11.zip
UploadPack v2 protocol: Stop negotiation for orphan refsstable-5.11
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
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java55
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java5
2 files changed, 60 insertions, 0 deletions
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 5045e9464e..3958de2d93 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 @@ public class UploadPackTest {
}
@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 9fda639280..63deff2d9e 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 @@ public class UploadPack {
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)

Back to the top