aboutsummaryrefslogtreecommitdiffstats
path: root/org.eclipse.jgit
diff options
context:
space:
mode:
authorPatrick Hiesel <hiesel@google.com>2022-05-13 13:49:02 +0200
committerIvan Frade <ifrade@google.com>2023-10-27 07:39:45 -0700
commit5f563e386ec9f696d9681720d8649b1e9b4adb7c (patch)
tree10e5db3518637c8c7031312940707671cf47d690 /org.eclipse.jgit
parent093bde518118175e1542fa2561f8d2f20649879b (diff)
downloadjgit-5f563e386ec9f696d9681720d8649b1e9b4adb7c.tar.gz
jgit-5f563e386ec9f696d9681720d8649b1e9b4adb7c.zip
UploadPack: use want-refs as advertised set in fetch v2
Protocol v2 introduced refs-in-wants and ls-remote with prefixes. UploadPack already uses prefixes provided by the client during a v2 ref advertisement (ls-refs). However, when the client consequently sends another request to fetch a previously advertised ref (with want-ref lines), the server uses the whole set of advertised refs to compute reachability. In repos with many refs, this slows down the reachability checks setting up and walking through unnecessary refs. For gerrit it can also break valid requests because in gerrit "all" means "recent" and the wanted-ref could fall out of the "recent" range when reloading all refs at fetch time. Treat wanted-refs like a ref-prefix when calculating the advertised refs on v2 fetch command. Less refs means a faster setup and less walk for the reachability checks. Note that wanted-refs filters only over the refs visible to the user, so this doesn't give any extra visibility to the caller. If the request contains also "want <oid>" lines, we cannot use this optimization. Those objects could be reachable from any visible branch, not necessarily in the wanted-refs. Google-Bug: b/122888978 Change-Id: I2a4ae171d4fc5d4cb30b020cb073ad23dd5a66c4
Diffstat (limited to 'org.eclipse.jgit')
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java23
1 files changed, 18 insertions, 5 deletions
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 33aa585569..9318871520 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1190,6 +1190,11 @@ public class UploadPack implements Closeable {
}
private void fetchV2(PacketLineOut pckOut) throws IOException {
+ ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig);
+ FetchV2Request req = parser.parseFetchRequest(pckIn);
+ currentRequest = req;
+ Map<String, ObjectId> wantedRefs = wantedRefs(req);
+
// Depending on the requestValidator, #processHaveLines may
// require that advertised be set. Set it only in the required
// circumstances (to avoid a full ref lookup in the case that
@@ -1199,16 +1204,25 @@ public class UploadPack implements Closeable {
requestValidator instanceof AnyRequestValidator) {
advertised = Collections.emptySet();
} else {
- advertised = refIdSet(getAdvertisedOrDefaultRefs().values());
+ if (req.wantIds.isEmpty()) {
+ // Only refs-in-wants in request. These ref-in-wants where used as
+ // filters already in the ls-refs, there is no need to use a full
+ // advertisement now in fetch. This improves performance and also
+ // accuracy: when the ref db prioritize and truncates the returned
+ // refs (e.g. Gerrit hides too old refs), applying a filter can
+ // return different results than a plain listing.
+ advertised = refIdSet(getFilteredRefs(wantedRefs.keySet()).values());
+ } else {
+ // At least one SHA1 in wants, so we need to take the full
+ // advertisement as base for a reachability check.
+ advertised = refIdSet(getAdvertisedOrDefaultRefs().values());
+ }
}
PackStatistics.Accumulator accumulator = new PackStatistics.Accumulator();
Instant negotiateStart = Instant.now();
accumulator.advertised = advertised.size();
- ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig);
- FetchV2Request req = parser.parseFetchRequest(pckIn);
- currentRequest = req;
rawOut.stopBuffering();
protocolV2Hook.onFetch(req);
@@ -1221,7 +1235,6 @@ public class UploadPack implements Closeable {
// copying data back to class fields
List<ObjectId> deepenNots = parseDeepenNots(req.getDeepenNots());
- Map<String, ObjectId> wantedRefs = wantedRefs(req);
// TODO(ifrade): Avoid mutating the parsed request.
req.getWantIds().addAll(wantedRefs.values());
wantIds = req.getWantIds();