aboutsummaryrefslogtreecommitdiffstats
path: root/org.eclipse.jgit.test
diff options
context:
space:
mode:
authorShawn Pearce <spearce@spearce.org>2017-09-06 19:09:42 -0700
committerShawn Pearce <spearce@spearce.org>2017-09-07 05:39:47 -0400
commite68a9b3ed8dbed4708f90d97ab2747c97aa0e123 (patch)
treef3a33314030513275ad0edadd953b9e497680e45 /org.eclipse.jgit.test
parentd0d15c38485011d32db2be1acf440110d20d6a1f (diff)
downloadjgit-e68a9b3ed8dbed4708f90d97ab2747c97aa0e123.tar.gz
jgit-e68a9b3ed8dbed4708f90d97ab2747c97aa0e123.zip
ReceivePack: clear advertised .haves if application changes refs
An application can choose to invoke setAdvertisedRefs multiple times, for example several AdvertiseRefsHook installed in a chain. Each of these invocations populates the advertisedHaves collection with the unique set of ObjectIds. This can lead to a server over-advertising with ".have" lines if the first hook pushes in a lot of references, and the second hook filters this to a subset. ReceivePack will advertise the unique objects from the first hook using ".have" lines, which may lead to a huge advertisement sent to the client. This can also contribute to a very slow connectivity check after the pack is parsed as ReceivePack calls markUninteresting on every commit in advertisedHaves. This may require expanding a lot of subtrees to mark all trees as uninteresting as well. On a very big repository this can lead to a many-second stall. Clear the advertisedHaves collection any time the refs are updated. Add a test to verify the correct set of objects was sent. Change-Id: I97f6998d0597251444a2e846a3ea1f461bae96f9
Diffstat (limited to 'org.eclipse.jgit.test')
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java41
1 files changed, 41 insertions, 0 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
index 3411122888..8ef87cb3c1 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
@@ -58,6 +58,8 @@ import java.security.MessageDigest;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.zip.Deflater;
import org.eclipse.jgit.errors.MissingObjectException;
@@ -159,6 +161,45 @@ public class ReceivePackAdvertiseRefsHookTest extends LocalDiskRepositoryTestCas
}
@Test
+ public void resetsHaves() throws Exception {
+ AtomicReference<Set<ObjectId>> haves = new AtomicReference<>();
+ try (TransportLocal t = new TransportLocal(src, uriOf(dst),
+ dst.getDirectory()) {
+ @Override
+ ReceivePack createReceivePack(Repository db) {
+ dst.incrementOpen();
+
+ ReceivePack rp = super.createReceivePack(dst);
+ rp.setAdvertiseRefsHook(new AdvertiseRefsHook() {
+ @Override
+ public void advertiseRefs(BaseReceivePack rp2)
+ throws ServiceMayNotContinueException {
+ rp.setAdvertisedRefs(rp.getRepository().getAllRefs(),
+ null);
+ new HidePrivateHook().advertiseRefs(rp);
+ haves.set(rp.getAdvertisedObjects());
+ }
+
+ @Override
+ public void advertiseRefs(UploadPack uploadPack)
+ throws ServiceMayNotContinueException {
+ throw new UnsupportedOperationException();
+ }
+ });
+ return rp;
+ }
+ }) {
+ try (PushConnection c = t.openPush()) {
+ // Just has to open/close for advertisement.
+ }
+ }
+
+ assertEquals(1, haves.get().size());
+ assertTrue(haves.get().contains(B));
+ assertFalse(haves.get().contains(P));
+ }
+
+ @Test
public void testSuccess() throws Exception {
// Manually force a delta of an object so we reuse it later.
//