From 2793c58dae47d1f96d642ec56e197ba2eb5af5b4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 13 Aug 2014 22:06:49 -0700 Subject: [PATCH] Update JGit to version 3.5.1.201410131835-r This JGit version fixes: - Bug 420915 - jgit gc hangs in partitionTasks with a very small repo - Bug 427107 - cannot push anymore The latter was observed by CollabNet to break Gerrit replication if gc created a bitmap index which may have induced PackWriterBitmapWalker. findObjects() to throw a MissingObjectException. This version of JGit also fixes the recursive merger on all storage systems. Objects created during the virtual base construction of a recursive merge must be written out somewhere and made available through an ObjectReader for later passes to work on. In both local filesystem and DFS implementations Gerrit was no-op'ing the inserter in dry-run mode, causing these objects to be lost and unavailable during the later processing stages of the merger. With a virtual common ancestor tree or blob missing, the dry-run merger fails and a spurious merge conflict is reported. Instead build a non-flushing inserter wrapper around a real inserter for the repository. On local disk (standard storage) this will allow the virtual base to write loose objects, which may be reclaimed in about two weeks by the standard `git prune` invoked by `git gc`. On DFS systems this will create a new pack file and buffer a block of data in memory before starting to store to persistent storage. However with no flush() the DfsInserter will attempt to rollback the pack, which may allow the DFS system to reclaim its storage quickly. Some implementations of DFS may buffer even more deeply than one block, making the discard even cheaper for smaller merges. This update also fixes a potential infinite loop during object inflation within both the WindowCursor or DfsReader versions of ObjectReader. Inflation could get stuck if an object's compression stream within a pack ended at a very precise alignment with the cache block size. The alignment problem is very rare, as it has taken several years to identify and track down. Includes changes done in I9859bd073bd710424e12b8b091abb8278f4f9fcc on master. Change-Id: I898ad7d5e836ebae0f8f84b17d0ae74489479ef9 --- .../acceptance/git/HttpPushForReviewIT.java | 7 ++++++- .../server/change/ChangeKindCacheImpl.java | 2 +- .../google/gerrit/server/git/MergeUtil.java | 20 +++++++------------ lib/jgit/BUCK | 12 +++++------ 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/HttpPushForReviewIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/HttpPushForReviewIT.java index 465befde49..71f008a079 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/HttpPushForReviewIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/HttpPushForReviewIT.java @@ -15,13 +15,18 @@ package com.google.gerrit.acceptance.git; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; import org.junit.Before; import java.io.IOException; +import java.net.URISyntaxException; public class HttpPushForReviewIT extends AbstractPushForReview { @Before - public void selectHttpUrl() throws GitAPIException, IOException { + public void selectHttpUrl() throws GitAPIException, IOException, URISyntaxException { + CredentialsProvider.setDefault(new UsernamePasswordCredentialsProvider( + admin.username, admin.httpPassword)); selectProtocol(Protocol.HTTP); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java index 8e2b6ac5a8..82e0c91729 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java @@ -218,7 +218,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { // having the same tree as would exist when the prior commit is // cherry-picked onto the next commit's new first parent. ThreeWayMerger merger = MergeUtil.newThreeWayMerger( - key.repo, MergeUtil.createDryRunInserter(), key.strategyName); + key.repo, MergeUtil.createDryRunInserter(key.repo), key.strategyName); merger.setBase(prior.getParent(0)); if (merger.merge(next.getParent(0), prior) && merger.getResultTreeId().equals(next.getTree())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java index b4a1f0c17c..b35d7e478f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java @@ -393,7 +393,7 @@ public class MergeUtil { return false; } - final ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter()); + ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter(repo)); try { return m.merge(new AnyObjectId[] {mergeTip, toMerge}); } catch (LargeObjectException e) { @@ -442,8 +442,7 @@ public class MergeUtil { // that on the current merge tip. // try { - final ThreeWayMerger m = - newThreeWayMerger(repo, createDryRunInserter()); + ThreeWayMerger m = newThreeWayMerger(repo, createDryRunInserter(repo)); m.setBase(toMerge.getParent(0)); return m.merge(mergeTip, toMerge); } catch (IOException e) { @@ -470,12 +469,12 @@ public class MergeUtil { } } - public static ObjectInserter createDryRunInserter() { - return new ObjectInserter() { + public static ObjectInserter createDryRunInserter(Repository db) { + final ObjectInserter delegate = db.newObjectInserter(); + return new ObjectInserter.Filter() { @Override - public ObjectId insert(int objectType, long length, InputStream in) - throws IOException { - return idFor(objectType, length, in); + protected ObjectInserter delegate() { + return delegate; } @Override @@ -487,11 +486,6 @@ public class MergeUtil { public void flush() throws IOException { // Do nothing. } - - @Override - public void release() { - // Do nothing. - } }; } diff --git a/lib/jgit/BUCK b/lib/jgit/BUCK index 9161a7b0ce..f0ab6299a7 100644 --- a/lib/jgit/BUCK +++ b/lib/jgit/BUCK @@ -1,12 +1,12 @@ include_defs('//lib/maven.defs') -VERS = '3.4.0.201406110918-r' +VERS = '3.5.1.201410131835-r' maven_jar( name = 'jgit', id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS, - bin_sha1 = '60e74a29895be82ec7bd1fbb6304975e92b955a5', - src_sha1 = '69adaa263e2b5c21a84a25105c0c93761bdd8a80', + bin_sha1 = '23b8793639407fcbe2fee8557fb8238d18b2e409', + src_sha1 = '48ae55f9fed45e188177bcf3bf4638eed6bf3aae', license = 'jgit', unsign = True, deps = [':ewah'], @@ -20,7 +20,7 @@ maven_jar( maven_jar( name = 'jgit-servlet', id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS, - sha1 = '5c778052a90520a970041f2f51e89cac9cf2cb3f', + sha1 = 'ee4f9852eb62d9b0785705e4eb40e40035119da7', license = 'jgit', deps = [':jgit'], unsign = True, @@ -33,7 +33,7 @@ maven_jar( maven_jar( name = 'jgit-archive', id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS, - sha1 = '8644e0dde6127c2d1b5a4fa902bf9b534764e969', + sha1 = '48ddea0e3f6e78f9696e30dfc257118a446b453c', license = 'jgit', deps = [':jgit', '//lib/commons:compress', @@ -49,7 +49,7 @@ maven_jar( maven_jar( name = 'junit', id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS, - sha1 = '4d1232e8b2412521d707d741ca14db9fc6806db2', + sha1 = '1e8a9e7fa493e96ec6c07b9f6f51ed2d2db60b9f', license = 'DO_NOT_DISTRIBUTE', unsign = True, deps = [':jgit'],