diff options
author | Shawn Pearce <spearce@spearce.org> | 2017-09-06 19:09:42 -0700 |
---|---|---|
committer | Shawn Pearce <spearce@spearce.org> | 2017-09-07 05:39:47 -0400 |
commit | e68a9b3ed8dbed4708f90d97ab2747c97aa0e123 (patch) | |
tree | f3a33314030513275ad0edadd953b9e497680e45 /org.eclipse.jgit.test | |
parent | d0d15c38485011d32db2be1acf440110d20d6a1f (diff) | |
download | jgit-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.java | 41 |
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. // |