diff options
6 files changed, 54 insertions, 43 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcSinceBitmapStatisticsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcSinceBitmapStatisticsTest.java index 3cd766c4e9..af52e2cb85 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcSinceBitmapStatisticsTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcSinceBitmapStatisticsTest.java @@ -20,6 +20,7 @@ import java.util.stream.StreamSupport; import org.eclipse.jgit.internal.storage.file.GC.RepoStatistics; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.pack.PackConfig; import org.junit.Test; public class GcSinceBitmapStatisticsTest extends GcTestCase { @@ -51,11 +52,19 @@ public class GcSinceBitmapStatisticsTest extends GcTestCase { } @Test - public void testShouldReportNoPacksDirectlyAfterGc() throws Exception { - // given + public void testShouldReportNoPacksFilesSinceBitmapWhenPackfilesAreOlderThanBitmapFile() + throws Exception { addCommit(null); - gc.gc().get(); + configureGC(/* buildBitmap */ false).gc().get(); + assertEquals(1L, gc.getStatistics().numberOfPackFiles); + assertEquals(0L, repositoryBitmapFiles()); + assertEquals(1L, gc.getStatistics().numberOfPackFilesSinceBitmap); + + addCommit(null); + configureGC(/* buildBitmap */ true).gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + assertEquals(2L, gc.getStatistics().numberOfPackFiles); assertEquals(0L, gc.getStatistics().numberOfPackFilesSinceBitmap); } @@ -79,8 +88,11 @@ public class GcSinceBitmapStatisticsTest extends GcTestCase { // progress & pack addCommit(parent); - tr.packAndPrune(); + assertEquals(1L, gc.getStatistics().numberOfPackFiles); + assertEquals(0L, gc.getStatistics().numberOfPackFilesSinceBitmap); + tr.packAndPrune(); + assertEquals(2L, gc.getStatistics().numberOfPackFiles); assertEquals(1L, gc.getStatistics().numberOfPackFilesSinceBitmap); } @@ -90,13 +102,17 @@ public class GcSinceBitmapStatisticsTest extends GcTestCase { // commit & gc RevCommit parent = addCommit(null); gc.gc().get(); + assertEquals(0L, gc.getStatistics().numberOfLooseObjects); assertEquals(0L, gc.getStatistics().numberOfObjectsSinceBitmap); // progress & pack addCommit(parent); + assertEquals(1L, gc.getStatistics().numberOfLooseObjects); assertEquals(1L, gc.getStatistics().numberOfObjectsSinceBitmap); tr.packAndPrune(); + assertEquals(0L, gc.getStatistics().numberOfLooseObjects); + // Number of objects contained in the newly created PackFile assertEquals(3L, gc.getStatistics().numberOfObjectsSinceBitmap); } @@ -164,4 +180,11 @@ public class GcSinceBitmapStatisticsTest extends GcTestCase { } }).sum(); } + + private GC configureGC(boolean buildBitmap) { + PackConfig pc = new PackConfig(repo.getObjectDatabase().getConfig()); + pc.setBuildBitmaps(buildBitmap); + gc.setPackConfig(pc); + return gc; + } } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 11435b8ce4..fd0995a77b 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -64,8 +64,6 @@ binaryHunkDecodeError=Binary hunk, line {0}: invalid input binaryHunkInvalidLength=Binary hunk, line {0}: input corrupt; expected length byte, got 0x{1} binaryHunkLineTooShort=Binary hunk, line {0}: input ended prematurely binaryHunkMissingNewline=Binary hunk, line {0}: input line not terminated by newline -bitmapAccessErrorForPackfile=Error whilst trying to access bitmap file for {} -bitmapFailedToGet=Failed to get bitmap index file {} bitmapMissingObject=Bitmap at {0} is missing {1}. bitmapsMustBePrepared=Bitmaps must be prepared before they may be written. bitmapUseNoopNoListener=Use NOOP instance for no listener diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 310962b967..c3091828ac 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -94,8 +94,6 @@ public class JGitText extends TranslationBundle { /***/ public String binaryHunkInvalidLength; /***/ public String binaryHunkLineTooShort; /***/ public String binaryHunkMissingNewline; - /***/ public String bitmapAccessErrorForPackfile; - /***/ public String bitmapFailedToGet; /***/ public String bitmapMissingObject; /***/ public String bitmapsMustBePrepared; /***/ public String bitmapUseNoopNoListener; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index f87ca90860..e61e2a712f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1579,7 +1579,7 @@ public class GC { public RepoStatistics getStatistics() throws IOException { RepoStatistics ret = new RepoStatistics(); Collection<Pack> packs = repo.getObjectDatabase().getPacks(); - long latestBitmapTime = Long.MIN_VALUE; + long latestBitmapTime = 0L; for (Pack p : packs) { long packedObjects = p.getIndex().getObjectCount(); ret.numberOfPackedObjects += packedObjects; @@ -1587,9 +1587,11 @@ public class GC { ret.sizeOfPackedObjects += p.getPackFile().length(); if (p.getBitmapIndex() != null) { ret.numberOfBitmaps += p.getBitmapIndex().getBitmapCount(); - latestBitmapTime = p.getFileSnapshot().lastModifiedInstant() - .toEpochMilli(); - } else { + if (latestBitmapTime == 0L) { + latestBitmapTime = p.getFileSnapshot().lastModifiedInstant().toEpochMilli(); + } + } + else if (latestBitmapTime == 0L) { ret.numberOfPackFilesSinceBitmap++; ret.numberOfObjectsSinceBitmap += packedObjects; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java index 1a7b5de1d7..5813d39e9a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java @@ -116,7 +116,7 @@ public class Pack implements Iterable<PackIndex.MutableEntry> { private volatile Exception invalidatingCause; @Nullable - private PackFile bitmapIdxFile; + private volatile PackFile bitmapIdxFile; private AtomicInteger transientErrorCount = new AtomicInteger(); @@ -694,7 +694,7 @@ public class Pack implements Iterable<PackIndex.MutableEntry> { private void doOpen() throws IOException { if (invalid) { - openFail(true, invalidatingCause); + openFail(invalidatingCause); throw new PackInvalidException(packFile, invalidatingCause); } try { @@ -705,39 +705,41 @@ public class Pack implements Iterable<PackIndex.MutableEntry> { } } catch (InterruptedIOException e) { // don't invalidate the pack, we are interrupted from another thread - openFail(false, e); + openFail(e); throw e; } catch (FileNotFoundException fn) { - // don't invalidate the pack if opening an existing file failed - // since it may be related to a temporary lack of resources (e.g. - // max open files) - openFail(!packFile.exists(), fn); + if (!packFile.exists()) { + // Failure to open an existing file may be related to a temporary lack of resources + // (e.g. max open files) + invalid = true; + } + openFail(fn); throw fn; } catch (EOFException | AccessDeniedException | NoSuchFileException | CorruptObjectException | NoPackSignatureException | PackMismatchException | UnpackException | UnsupportedPackIndexVersionException | UnsupportedPackVersionException pe) { - // exceptions signaling permanent problems with a pack - openFail(true, pe); + invalid = true; // exceptions signaling permanent problems with a pack + openFail(pe); throw pe; } catch (IOException ioe) { - // mark this packfile as invalid when NFS stale file handle error - // occur - openFail(FileUtils.isStaleFileHandleInCausalChain(ioe), ioe); + if (FileUtils.isStaleFileHandleInCausalChain(ioe)) { + invalid = true; + } + openFail(ioe); throw ioe; } catch (RuntimeException ge) { // generic exceptions could be transient so we should not mark the // pack invalid to avoid false MissingObjectExceptions - openFail(false, ge); + openFail(ge); throw ge; } } - private void openFail(boolean invalidate, Exception cause) { + private void openFail(Exception cause) { activeWindows = 0; activeCopyRawData = 0; - invalid = invalidate; invalidatingCause = cause; doClose(); } @@ -1211,17 +1213,8 @@ public class Pack implements Iterable<PackIndex.MutableEntry> { return null; } - synchronized void refreshBitmapIndex(PackFile bitmapIndexFile) { - this.bitmapIdx = Optionally.empty(); - this.invalid = false; + void setBitmapIndexFile(PackFile bitmapIndexFile) { this.bitmapIdxFile = bitmapIndexFile; - try { - getBitmapIndex(); - } catch (IOException e) { - LOG.warn(JGitText.get().bitmapFailedToGet, bitmapIdxFile, e); - this.bitmapIdx = Optionally.empty(); - this.bitmapIdxFile = null; - } } private synchronized PackReverseIndex getReverseIdx() throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java index e31126f027..5cc7be4f98 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java @@ -459,12 +459,9 @@ class PackDirectory { && !oldPack.getFileSnapshot().isModified(packFile)) { forReuse.remove(packFile.getName()); list.add(oldPack); - try { - if(oldPack.getBitmapIndex() == null) { - oldPack.refreshBitmapIndex(packFilesByExt.get(BITMAP_INDEX)); - } - } catch (IOException e) { - LOG.warn(JGitText.get().bitmapAccessErrorForPackfile, oldPack.getPackName(), e); + PackFile bitMaps = packFilesByExt.get(BITMAP_INDEX); + if (bitMaps != null) { + oldPack.setBitmapIndexFile(bitMaps); } continue; } |