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 @@ package org.eclipse.jgit.internal.storage.file; 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.Future; 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 class GcConcurrentTest extends GcTestCase { 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 @@ public class PackFile implements Iterable { 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))