From 1ed1e40387c859786cf1932d22fe0001fbd5abb6 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Mon, 7 Aug 2017 14:26:46 +0200 Subject: [PATCH 1/2] Fix exception handling for opening bitmap index files When creating a new PackFile instance it is specified whether this pack has an associated bitmap index file or not. This information is cached and the public method getBitmapIndex() will always assume a bitmap index file must exist if the cached data tells so. But it may happen that the packfiles are repacked during a gc in a different process causing the packfile, bitmap-index and index file to be deleted. Since JGit still has an open FileHandle on the packfile this file is not really deleted and can still be accessed. But index and bitmap index file are deleted. Fix getBitmapIndex() to invalidate the cached packfile instance if such a situation occurs. This problem showed up when a gerrit server was serving repositories which where garbage collected with native git regularly. Fetch and clone commands for certain repositories failed permanently after a native git gc had deleted old bitmap index files. Change-Id: I8e620bec74dd3f310ba42024f9a657062f868f0e Signed-off-by: Matthias Sohn --- .../storage/file/GcConcurrentTest.java | 103 ++++++++++++++++++ .../jgit/internal/storage/file/PackFile.java | 13 ++- 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java index 07a7be746..776226b4e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java @@ -45,8 +45,12 @@ import static java.lang.Integer.valueOf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; import java.util.concurrent.CyclicBarrier; @@ -56,8 +60,14 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.internal.storage.pack.PackWriter; +import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.EmptyProgressMonitor; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Sets; import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Test; public class GcConcurrentTest extends GcTestCase { @@ -116,4 +126,97 @@ public Integer call() throws Exception { pool.awaitTermination(Long.MAX_VALUE, TimeUnit.SECONDS); } } + + @Test + public void repackAndGetStats() throws Exception { + TestRepository.BranchBuilder test = tr.branch("test"); + test.commit().add("a", "a").create(); + GC gc1 = new GC(tr.getRepository()); + gc1.setPackExpireAgeMillis(0); + gc1.gc(); + test.commit().add("b", "b").create(); + + // Create a new Repository instance and trigger a gc + // from that instance. Reusing the existing repo instance + // tr.getRepository() would not show the problem. + FileRepository r2 = new FileRepository( + tr.getRepository().getDirectory()); + GC gc2 = new GC(r2); + gc2.setPackExpireAgeMillis(0); + gc2.gc(); + + new GC(tr.getRepository()).getStatistics(); + } + + @Test + public void repackAndUploadPack() throws Exception { + TestRepository.BranchBuilder test = tr.branch("test"); + // RevCommit a = test.commit().add("a", "a").create(); + test.commit().add("a", "a").create(); + + GC gc1 = new GC(tr.getRepository()); + gc1.setPackExpireAgeMillis(0); + gc1.gc(); + + RevCommit b = test.commit().add("b", "b").create(); + + FileRepository r2 = new FileRepository( + tr.getRepository().getDirectory()); + GC gc2 = new GC(r2); + gc2.setPackExpireAgeMillis(0); + gc2.gc(); + + // Simulate parts of an UploadPack. This is the situation on + // server side (e.g. gerrit) when when clients are + // cloning/fetching while the server side repo's + // are gc'ed by an external process (e.g. scheduled + // native git gc) + try (PackWriter pw = new PackWriter(tr.getRepository())) { + pw.setUseBitmaps(true); + pw.preparePack(NullProgressMonitor.INSTANCE, Sets.of(b), + Collections. emptySet()); + new GC(tr.getRepository()).getStatistics(); + } + } + + PackFile getSinglePack(FileRepository r) { + Collection packs = r.getObjectDatabase().getPacks(); + assertEquals(1, packs.size()); + return packs.iterator().next(); + } + + @Test + public void repackAndCheckBitmapUsage() throws Exception { + // create a test repository with one commit and pack all objects. After + // packing create loose objects to trigger creation of a new packfile on + // the next gc + TestRepository.BranchBuilder test = tr.branch("test"); + test.commit().add("a", "a").create(); + FileRepository repository = tr.getRepository(); + GC gc1 = new GC(repository); + gc1.setPackExpireAgeMillis(0); + gc1.gc(); + String oldPackName = getSinglePack(repository).getPackName(); + RevCommit b = test.commit().add("b", "b").create(); + + // start the garbage collection on a new repository instance, + FileRepository repository2 = new FileRepository(repository.getDirectory()); + GC gc2 = new GC(repository2); + gc2.setPackExpireAgeMillis(0); + gc2.gc(); + String newPackName = getSinglePack(repository2).getPackName(); + // make sure gc() has caused creation of a new packfile + assertNotEquals(oldPackName, newPackName); + + // Even when asking again for the set of packfiles outdated data + // will be returned. As long as the repository can work on cached data + // it will do so and not detect that a new packfile exists. + assertNotEquals(getSinglePack(repository).getPackName(), newPackName); + + // Only when accessing object content it is required to rescan the pack + // directory and the new packfile will be detected. + repository.getObjectDatabase().open(b).getSize(); + assertEquals(getSinglePack(repository).getPackName(), newPackName); + assertNotNull(getSinglePack(repository).getBitmapIndex()); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index 038172f92..b5889f2dd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -1105,8 +1105,17 @@ synchronized PackBitmapIndex getBitmapIndex() throws IOException { if (invalid || invalidBitmap) return null; if (bitmapIdx == null && hasExt(BITMAP_INDEX)) { - final PackBitmapIndex idx = PackBitmapIndex.open( - extFile(BITMAP_INDEX), idx(), getReverseIdx()); + final PackBitmapIndex idx; + try { + idx = PackBitmapIndex.open(extFile(BITMAP_INDEX), idx(), + getReverseIdx()); + } catch (FileNotFoundException e) { + // Once upon a time this bitmap file existed. Now it + // has been removed. Most likely an external gc has + // removed this packfile and the bitmap + invalidBitmap = true; + return null; + } // At this point, idx() will have set packChecksum. if (Arrays.equals(packChecksum, idx.packChecksum)) From d9fd35281b1817d349b5142104dd6c2428c6a548 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 14 Aug 2017 23:37:21 +0200 Subject: [PATCH 2/2] Update Oxygen Orbit p2 repository to R20170516192513 Change-Id: I7f1b733ca414d77c9df5572df02e742e0a84ba2f Signed-off-by: Matthias Sohn --- .../org.eclipse.jgit.target/jgit-4.5.target | 4 ++-- .../org.eclipse.jgit.target/jgit-4.5.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.6.target | 4 ++-- .../org.eclipse.jgit.target/jgit-4.6.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.7.target | 4 ++-- .../org.eclipse.jgit.target/jgit-4.7.tpd | 2 +- ...{S20170120205402-Oxygen.tpd => R20170516192513-Oxygen.tpd} | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) rename org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/{S20170120205402-Oxygen.tpd => R20170516192513-Oxygen.tpd} (94%) diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target index 85585e804..d0da0c415 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target @@ -1,7 +1,7 @@ - + @@ -98,7 +98,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd index 77d3e89ee..efc1f4461 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd @@ -2,7 +2,7 @@ target "jgit-4.5" with source configurePhase include "projects/jetty-9.3.17.tpd" include "orbit/R20160221192158-Mars.tpd" -include "orbit/S20170120205402-Oxygen.tpd" +include "orbit/R20170516192513-Oxygen.tpd" location "http://download.eclipse.org/releases/mars/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target index a51b4559d..065284de1 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.target @@ -1,7 +1,7 @@ - + @@ -60,7 +60,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd index 913f66124..90f62ae4c 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.6.tpd @@ -1,7 +1,7 @@ target "jgit-4.6" with source configurePhase include "projects/jetty-9.3.17.tpd" -include "orbit/S20170120205402-Oxygen.tpd" +include "orbit/R20170516192513-Oxygen.tpd" location "http://download.eclipse.org/releases/neon/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target index 541ce04ce..d3a11314f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.target @@ -1,7 +1,7 @@ - + @@ -60,7 +60,7 @@ - + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd index 0ce8a9a0b..1d0e69364 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.7.tpd @@ -1,7 +1,7 @@ target "jgit-4.7" with source configurePhase include "projects/jetty-9.3.17.tpd" -include "orbit/S20170120205402-Oxygen.tpd" +include "orbit/R20170516192513-Oxygen.tpd" location "http://download.eclipse.org/releases/oxygen/" { org.eclipse.osgi lazy diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20170120205402-Oxygen.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20170516192513-Oxygen.tpd similarity index 94% rename from org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20170120205402-Oxygen.tpd rename to org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20170516192513-Oxygen.tpd index b369cb637..360062838 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20170120205402-Oxygen.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20170516192513-Oxygen.tpd @@ -1,7 +1,7 @@ -target "S20170120205402-Oxygen" with source configurePhase +target "R20170516192513-Oxygen" with source configurePhase // see http://download.eclipse.org/tools/orbit/downloads/ -location "http://download.eclipse.org/tools/orbit/downloads/drops/S20170120205402/repository" { +location "http://download.eclipse.org/tools/orbit/R-builds/R20170516192513/repository" { org.apache.ant [1.9.6.v201510161327,1.9.6.v201510161327] org.apache.ant.source [1.9.6.v201510161327,1.9.6.v201510161327] org.apache.commons.compress [1.6.0.v201310281400,1.6.0.v201310281400]