motiejus
/
jgit
1
Fork 0
Code Releases Activity

BasePackFetchConnection: support negotiationTip feature 

By default, Git will report, to the server, commits reachable from all local refs to find common commits in an attempt to reduce the size of the to-be-received packfile. If specified with negotiation tip, Git will only report commits reachable from the given tips. This is useful to speed up fetches when the user knows which local ref is likely to have commits in common with the upstream ref being fetched.

When negotation-tip is on, use the wanted refs instead of all refs as source of the "have" list to send.

This is controlled by the `fetch.usenegotationtip` flag, false by default. This works only for programmatic fetches and there is no support for it yet in the CLI.

Change-Id: I19f8fe48889bfe0ece7cdf78019b678ede5c6a32
This commit is contained in:
Ronald Bhuleskar 2023-03-22 15:07:19 -07:00
parent 5166ded098
commit 738dacb7fb
3 changed files with 274 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/config-options.md
View File

@ -50,6 +50,12 @@ For details on native git options see also the official [git config documentatio
| `core.trustPackedRefsStat` | `unset` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the packed-refs file. If `never` JGit will ignore the file attributes of the packed-refs file and always read it. If `always` JGit will trust the file attributes of the packed-refs file and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, except that the packed-refs file is opened and closed before its file attributes are considered. An open/close of the packed-refs file is known to refresh its file attributes, at least on some NFS clients. If `unset`, JGit will use the behavior described in `trustFolderStat`. |
| `core.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | ✅ | The path to the root of the working tree. |
## __fetch__ options
| option | default | git option | description |
|---------|---------|------------|-------------|
| `fetch.useNegotiationTip` | `false` | ✅ | When enabled it restricts the client negotiation on unrelated branches i.e. only send haves for the refs that the client is interested in fetching. |
## __gc__ options
| option | default | git option | description |

232
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
View File

@ -22,9 +22,12 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import org.eclipse.jgit.dircache.DirCache;
@ -53,6 +56,7 @@
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.storage.pack.PackStatistics;
import org.eclipse.jgit.transport.BasePackFetchConnection.FetchConfig;
import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
import org.eclipse.jgit.util.io.NullOutputStream;
import org.junit.After;
@ -63,6 +67,8 @@
* Tests for server upload-pack utilities.
*/
public class UploadPackTest {
private static final int MAX_HAVES = 64;
private URIish uri;
private TestProtocol<Object> testProtocol;
@ -87,6 +93,7 @@ public void setUp() throws Exception {
@After
public void tearDown() {
TestProtocol.setFetchConfig(null);
Transport.unregister(testProtocol);
}
@ -2158,6 +2165,231 @@ public void testV2FetchWantRef() throws Exception {
assertFalse(client.getObjectDatabase().has(three.toObjectId()));
}
/**
* <pre>
* remote:
* foo <- foofoo <-- branchFoo
* bar <- barbar <-- branchBar
*
* client:
* foo <-- branchFoo
* bar <-- branchBar
*
* fetch(branchFoo) should send exactly 1 have (i.e. foo) from branchFoo
* </pre>
*/
@Test
public void testNegotiationTip() throws Exception {
RevCommit fooParent = remote.commit().message("foo").create();
RevCommit fooChild = remote.commit().message("foofoo").parent(fooParent)
.create();
RevCommit barParent = remote.commit().message("bar").create();
RevCommit barChild = remote.commit().message("barbar").parent(barParent)
.create();
// Remote has branchFoo at fooChild and branchBar at barChild
remote.update("branchFoo", fooChild);
remote.update("branchBar", barChild);
AtomicReference<UploadPack> uploadPack = new AtomicReference<>();
CountHavesPreUploadHook countHavesHook = new CountHavesPreUploadHook();
RevCommit localFooParent = null;
// Client has lagging branchFoo at fooParent and branchBar at barParent
try (TestRepository<InMemoryRepository> clientRepo = new TestRepository<>(
client)) {
localFooParent = clientRepo.commit().message("foo")
.create();
RevCommit localBarParent = clientRepo.commit().message("bar")
.create();
clientRepo.update("branchFoo", localFooParent);
clientRepo.update("branchBar", localBarParent);
testProtocol = new TestProtocol<>((Object req, Repository db) -> {
UploadPack up = new UploadPack(db);
up.setPreUploadHook(countHavesHook);
uploadPack.set(up);
return up;
}, null);
uri = testProtocol.register(ctx, server);
TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES, true));
try (Transport tn = testProtocol.open(uri,
clientRepo.getRepository(), "server")) {
tn.fetch(NullProgressMonitor.INSTANCE, Collections
.singletonList(new RefSpec("refs/heads/branchFoo")),
"branchFoo");
}
}
assertTrue(client.getObjectDatabase().has(fooParent.toObjectId()));
assertTrue(client.getObjectDatabase().has(fooChild.toObjectId()));
assertFalse(client.getObjectDatabase().has(barChild.toObjectId()));
assertEquals(1, uploadPack.get().getStatistics().getHaves());
assertTrue(countHavesHook.havesSentDuringNegotiation
.contains(localFooParent.toObjectId()));
}
/**
* Remote has 2 branches branchFoo and branchBar, each of them have 128 (2 x
* MAX_HAVES) commits each. Local has both the branches with lagging
* commits. Only 64 (1 x MAX_HAVES) from each branch and lagging 64.
* fetch(branchFoo) should send all 64 (MAX_HAVES) commits on branchFoo as
* haves and none from branchBar.
*
* Visual representation of the same:
*
* <pre>
* remote:
* parent
* / \
* branchFoo-0 branchBar-0
* branchFoo-1 branchBar-1
* ... ...
* ... ...
* branchFoo-128 branchBar-128
* ^-- branchFoo ^--branchBar
*
* local:
* parent
* / \
* branchFoo-0 branchBar-0
* branchFoo-1 branchBar-1
* ... ...
* ... ...
* branchFoo-64 branchBar-64
* ^-- branchFoo ^--branchBar
*
* fetch(branchFoo) should send all 64 (MAX_HAVES) commits on branchFoo as haves
* </pre>
*/
@Test
public void testNegotiationTipWithLongerHistoryThanMaxHaves()
throws Exception {
Set<RevCommit> remoteFooCommits = new HashSet<>();
Set<RevCommit> remoteBarCommits = new HashSet<>();
RevCommit parent = remote.commit().message("branchFoo-0").create();
RevCommit parentFoo = parent;
remoteFooCommits.add(parentFoo);
for (int i = 1; i < 2 * MAX_HAVES; i++) {
RevCommit child = remote.commit()
.message("branchFoo-" + Integer.toString(i))
.parent(parentFoo)
.create();
parentFoo = child;
remoteFooCommits.add(parentFoo);
}
remote.update("branchFoo", parentFoo);
RevCommit parentBar = parent;
remoteBarCommits.add(parentBar);
for (int i = 1; i < 2 * MAX_HAVES; i++) {
RevCommit child = remote.commit()
.message("branchBar-" + Integer.toString(i))
.parent(parentBar)
.create();
parentBar = child;
remoteBarCommits.add(parentBar);
}
remote.update("branchBar", parentBar);
AtomicReference<UploadPack> uploadPack = new AtomicReference<>();
CountHavesPreUploadHook countHavesHook = new CountHavesPreUploadHook();
Set<ObjectId> localFooCommits = new HashSet<>();
try (TestRepository<InMemoryRepository> clientRepo = new TestRepository<>(
client)) {
RevCommit localParent = clientRepo.commit().message("branchBar-0")
.create();
RevCommit localParentFoo = localParent;
localFooCommits.add(localParentFoo);
for (int i = 1; i < 1 * MAX_HAVES; i++) {
RevCommit child = clientRepo.commit()
.message("branchFoo-" + Integer.toString(i))
.parent(localParentFoo).create();
localParentFoo = child;
localFooCommits.add(localParentFoo);
}
clientRepo.update("branchFoo", localParentFoo);
RevCommit localParentBar = localParent;
for (int i = 1; i < 1 * MAX_HAVES; i++) {
RevCommit child = clientRepo.commit()
.message("branchBar-" + Integer.toString(i))
.parent(localParentBar).create();
localParentBar = child;
}
clientRepo.update("branchBar", localParentBar);
testProtocol = new TestProtocol<>((Object req, Repository db) -> {
UploadPack up = new UploadPack(db);
up.setPreUploadHook(countHavesHook);
uploadPack.set(up);
return up;
}, null);
uri = testProtocol.register(ctx, server);
TestProtocol.setFetchConfig(new FetchConfig(true, MAX_HAVES, true));
try (Transport tn = testProtocol.open(uri,
clientRepo.getRepository(), "server")) {
tn.fetch(NullProgressMonitor.INSTANCE, Collections
.singletonList(new RefSpec("refs/heads/branchFoo")),
"branchFoo");
}
}
for (RevCommit c : remoteFooCommits) {
assertTrue(c.toObjectId() + "",
client.getObjectDatabase().has(c.toObjectId()));
}
remoteBarCommits.remove(parent);
for (RevCommit c : remoteBarCommits) {
assertFalse(client.getObjectDatabase().has(c.toObjectId()));
}
assertEquals(MAX_HAVES, uploadPack.get().getStatistics().getHaves());
// Verify that all the haves that were sent during negotiation are local
// commits from branchFoo
for (Object id : countHavesHook.havesSentDuringNegotiation) {
assertTrue(localFooCommits.contains(id));
}
}
private static class CountHavesPreUploadHook implements PreUploadHook {
Set<ObjectId> havesSentDuringNegotiation = new HashSet<>();
@Override
public void onSendPack(UploadPack unusedUploadPack,
Collection<? extends ObjectId> unusedWants,
Collection<? extends ObjectId> haves)
throws ServiceMayNotContinueException {
// record haves
havesSentDuringNegotiation.addAll(haves);
}
@Override
public void onEndNegotiateRound(UploadPack unusedUploadPack,
Collection<? extends ObjectId> unusedWants, int unusedCntCommon,
int unusedCntNotFound, boolean unusedReady)
throws ServiceMayNotContinueException {
// Do nothing.
}
@Override
public void onBeginNegotiateRound(UploadPack unusedUploadPack,
Collection<? extends ObjectId> unusedWants,
int unusedCntOffered) throws ServiceMayNotContinueException {
// Do nothing.
}
}
@Test
public void testV2FetchBadWantRef() throws Exception {
RevCommit one = remote.commit().message("1").create();

38
org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java
View File

@ -37,6 +37,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.PackProtocolException;
import org.eclipse.jgit.errors.RemoteRepositoryException;
@ -215,6 +216,8 @@ public abstract class BasePackFetchConnection extends BasePackConnection
private boolean allowOfsDelta;
private boolean useNegotiationTip;
private boolean noDone;
private boolean noProgress;
@ -259,9 +262,11 @@ public BasePackFetchConnection(PackTransport packTransport) {
final FetchConfig cfg = getFetchConfig();
allowOfsDelta = cfg.allowOfsDelta;
maxHaves = cfg.maxHaves;
useNegotiationTip = cfg.useNegotiationTip;
} else {
allowOfsDelta = true;
maxHaves = Integer.MAX_VALUE;
useNegotiationTip = false;
}
includeTags = transport.getTagOpt() != TagOpt.NO_TAGS;
@ -297,14 +302,35 @@ static class FetchConfig {
final int maxHaves;
final boolean useNegotiationTip;
FetchConfig(Config c) {
allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); //$NON-NLS-1$ //$NON-NLS-2$
maxHaves = c.getInt("fetch", "maxhaves", Integer.MAX_VALUE); //$NON-NLS-1$ //$NON-NLS-2$
useNegotiationTip = c.getBoolean("fetch", "usenegotiationtip", //$NON-NLS-1$ //$NON-NLS-2$
false);
}
FetchConfig(boolean allowOfsDelta, int maxHaves) {
this(allowOfsDelta, maxHaves, false);
}
/**
* @param allowOfsDelta
* when true optimizes the pack size by deltafying base
* object
* @param maxHaves
* max haves to be sent per negotiation
* @param useNegotiationTip
* if true uses the wanted refs instead of all refs as source
* of the "have" list to send.
* @since 6.6
*/
FetchConfig(boolean allowOfsDelta, int maxHaves,
boolean useNegotiationTip) {
this.allowOfsDelta = allowOfsDelta;
this.maxHaves = maxHaves;
this.useNegotiationTip = useNegotiationTip;
}
}
@ -384,7 +410,7 @@ protected void doFetch(final ProgressMonitor monitor,
noProgress = monitor == NullProgressMonitor.INSTANCE;
markRefsAdvertised();
markReachable(have, maxTimeWanted(want));
markReachable(want, have, maxTimeWanted(want));
if (TransferConfig.ProtocolVersion.V2
.equals(getProtocolVersion())) {
@ -662,9 +688,17 @@ private int maxTimeWanted(Collection<Ref> wants) {
return maxTime;
}
private void markReachable(Set<ObjectId> have, int maxTime)
private void markReachable(Collection<Ref> want, Set<ObjectId> have,
int maxTime)
throws IOException {
Set<String> wantRefs = want.stream().map(Ref::getName)
.collect(Collectors.toSet());
for (Ref r : local.getRefDatabase().getRefs()) {
if (useNegotiationTip && !wantRefs.contains(r.getName())) {
continue;
}
ObjectId id = r.getPeeledObjectId();
if (id == null)
id = r.getObjectId();