From c398fb2b6b5be13251a74ba36b5fd5a9ef360a1a Mon Sep 17 00:00:00 2001 From: Minh Thai Date: Fri, 31 Aug 2018 15:54:39 -0700 Subject: [PATCH] Replace useminimalnegotation flag with maxhaves flag Client will stop sending haves when the number of haves sent reaches maxhaves. Change-Id: I1e5b1525be4c67f20a81ca24a2770c20eb5c1271 Signed-off-by: Minh Thai --- .../jgit/transport/TestProtocolTest.java | 22 ++------ .../transport/BasePackFetchConnection.java | 53 ++++--------------- 2 files changed, 15 insertions(+), 60 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java index 86c92bb52..953c9fc30 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java @@ -77,6 +77,7 @@ public class TestProtocolTest { "+refs/heads/master:refs/heads/master"); private static final int HAVES_PER_ROUND = 32; + private static final int MAX_HAVES = 256; private static class User { private final String name; @@ -187,7 +188,7 @@ public class TestProtocolTest { } @Test - public void testMinimalNegotiation() throws Exception { + public void testMaxHaves() throws Exception { TestProtocol proto = registerDefault(); URIish uri = proto.register(new User("user"), remote.getRepository()); @@ -200,28 +201,13 @@ public class TestProtocolTest { RevCommit master = remote.branch("master").commit() .add("readme.txt", "unique commit").create(); - TestProtocol.setFetchConfig(new FetchConfig(true, true)); + TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES)); try (Git git = new Git(local.getRepository())) { assertNull(local.getRepository().exactRef("refs/heads/master")); git.fetch().setRemote(uri.toString()).setRefSpecs(MASTER).call(); assertEquals(master, local.getRepository() .exactRef("refs/heads/master").getObjectId()); - assertTrue(havesCount <= 2 * HAVES_PER_ROUND); - - // Update the remote master and add local branches for 5 more rounds - // of negotiation, where the local branch commits have newer - // timestamps. Negotiation should send 5 rounds for those newer - // branches before getting to the round that sends its stale version - // of master. - master = remote.branch("master").commit().parent(master).create(); - local.tick(2 * HAVES_PER_ROUND); - for (int i = 0; i < 5 * HAVES_PER_ROUND; i++) { - local.branch("local-" + i).commit().create(); - } - git.fetch().setRemote(uri.toString()).setRefSpecs(MASTER).call(); - assertEquals(master, local.getRepository() - .exactRef("refs/heads/master").getObjectId()); - assertEquals(6 * HAVES_PER_ROUND, havesCount); + assertTrue(havesCount <= MAX_HAVES); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java index 0dfcd8716..ed7465c82 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java @@ -52,7 +52,6 @@ import java.text.MessageFormat; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashSet; import java.util.Set; import org.eclipse.jgit.errors.PackProtocolException; @@ -235,12 +234,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection private boolean noProgress; - private Set minimalNegotiationSet; - private String lockMessage; private PackLock packLock; + private int maxHaves; + /** RPC state, if {@link BasePackConnection#statelessRPC} is true. */ private TemporaryBuffer.Heap state; @@ -261,12 +260,12 @@ public abstract class BasePackFetchConnection extends BasePackConnection if (local != null) { final FetchConfig cfg = getFetchConfig(); allowOfsDelta = cfg.allowOfsDelta; - if (cfg.minimalNegotiation) { - minimalNegotiationSet = new HashSet<>(); - } + maxHaves = cfg.maxHaves; } else { allowOfsDelta = true; + maxHaves = Integer.MAX_VALUE; } + includeTags = transport.getTagOpt() != TagOpt.NO_TAGS; thinPack = transport.isFetchThin(); filterBlobLimit = transport.getFilterBlobLimit(); @@ -294,17 +293,16 @@ public abstract class BasePackFetchConnection extends BasePackConnection static class FetchConfig { final boolean allowOfsDelta; - final boolean minimalNegotiation; + final int maxHaves; FetchConfig(Config c) { allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); //$NON-NLS-1$ //$NON-NLS-2$ - minimalNegotiation = c.getBoolean("fetch", "useminimalnegotiation", //$NON-NLS-1$ //$NON-NLS-2$ - false); + maxHaves = c.getInt("fetch", "maxhaves", Integer.MAX_VALUE); //$NON-NLS-1$ //$NON-NLS-2$ } - FetchConfig(boolean allowOfsDelta, boolean minimalNegotiation) { + FetchConfig(boolean allowOfsDelta, int maxHaves) { this.allowOfsDelta = allowOfsDelta; - this.minimalNegotiation = minimalNegotiation; + this.maxHaves = maxHaves; } } @@ -518,15 +516,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection } line.append('\n'); p.writeString(line.toString()); - if (minimalNegotiationSet != null) { - Ref current = local.exactRef(r.getName()); - if (current != null) { - ObjectId o = current.getObjectId(); - if (o != null && !o.equals(ObjectId.zeroId())) { - minimalNegotiationSet.add(o); - } - } - } } if (first) { return false; @@ -610,9 +599,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection pckOut.writeString("have " + o.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ havesSent++; havesSinceLastContinue++; - if (minimalNegotiationSet != null) { - minimalNegotiationSet.remove(o); - } if ((31 & havesSent) != 0) { // We group the have lines into blocks of 32, each marked @@ -646,16 +632,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection // pack on the remote side. Keep doing that. // resultsPending--; - if (minimalNegotiationSet != null - && minimalNegotiationSet.isEmpty()) { - // Minimal negotiation was requested and we sent out our - // current reference values for our wants, so terminate - // negotiation early. - if (statelessRPC) { - state.writeTo(out, null); - } - break SEND_HAVES; - } break READ_RESULT; case ACK: @@ -686,14 +662,6 @@ public abstract class BasePackFetchConnection extends BasePackConnection if (anr == AckNackResult.ACK_READY) { receivedReady = true; } - if (minimalNegotiationSet != null && minimalNegotiationSet.isEmpty()) { - // Minimal negotiation was requested and we sent out our current reference - // values for our wants, so terminate negotiation early. - if (statelessRPC) { - state.writeTo(out, null); - } - break SEND_HAVES; - } break; } @@ -709,7 +677,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection state.writeTo(out, null); } - if (receivedContinue && havesSinceLastContinue > MAX_HAVES) { + if (receivedContinue && havesSinceLastContinue > MAX_HAVES + || havesSent >= maxHaves) { // Our history must be really different from the remote's. // We just sent a whole slew of have lines, and it did not // recognize any of them. Avoid sending our entire history