From 10135580d0842972c61daa3aa52493ea4c7d4485 Mon Sep 17 00:00:00 2001 From: Terry Parker Date: Fri, 25 Mar 2016 13:55:48 -0700 Subject: [PATCH 1/2] In TestRepository, use a consistent clock The default author and committer objects in TestRepository were initialized statically and did not use the MockSystemReader passed into the TestRepository ctor. Make these fields non-static and initialize them with a consistent clock. Also make the author and commiter name and email strings public for tests that want to verify against them. Change-Id: I88b444b96e22743001b32824d8e4e03c2239aa86 Signed-off-by: Terry Parker --- .../eclipse/jgit/junit/TestRepository.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index e259156c3..1119e6382 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -112,23 +112,18 @@ import org.eclipse.jgit.util.io.SafeBufferedOutputStream; * type of Repository the test data is stored on. */ public class TestRepository { - private static final PersonIdent defaultAuthor; - private static final PersonIdent defaultCommitter; + public static final String AUTHOR = "J. Author"; - static { - final MockSystemReader m = new MockSystemReader(); - final long now = m.getCurrentTime(); - final int tz = m.getTimezone(now); + public static final String AUTHOR_EMAIL = "jauthor@example.com"; - final String an = "J. Author"; - final String ae = "jauthor@example.com"; - defaultAuthor = new PersonIdent(an, ae, now, tz); + public static final String COMMITTER = "J. Committer"; - final String cn = "J. Committer"; - final String ce = "jcommitter@example.com"; - defaultCommitter = new PersonIdent(cn, ce, now, tz); - } + public static final String COMMITTER_EMAIL = "jcommitter@example.com"; + + private final PersonIdent defaultAuthor; + + private final PersonIdent defaultCommitter; private final R db; @@ -184,6 +179,10 @@ public class TestRepository { this.pool = rw; this.inserter = db.newObjectInserter(); this.mockSystemReader = reader; + long now = mockSystemReader.getCurrentTime(); + int tz = mockSystemReader.getTimezone(now); + defaultAuthor = new PersonIdent(AUTHOR, AUTHOR_EMAIL, now, tz); + defaultCommitter = new PersonIdent(COMMITTER, COMMITTER_EMAIL, now, tz); } /** @return the repository this helper class operates against. */ From 74743bc547946bf5730b5caa45645804b2226d35 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Tue, 29 Mar 2016 16:41:07 +0200 Subject: [PATCH 2/2] Add config parameter gc.prunePackExpire for packfile expiration JGit's Garbage Collector is repacking relevant objects into new packfiles and is afterwards deleting the now obsolete packfiles. But to prevent problems caused by race conditions JGit was not deleting packfiles when they are too young. The same mechanism as for loose objects and the config parameter gc.pruneExpire was used. But JGit was reusing the parameter gc.pruneExpire also for packfiles which may cause a lot of filesystem consumption if gc.pruneExpire was set to the default of 2 weeks. Only two weeks after packfile creation gc was allowed to delete this packfile. This change introduces a new config paramter gc.prunePackExpire with a default of "1.hour". This parameter is used when packfiles are deleted. Only packfiles older than the specified time can be deleted. For loose objects the behaviour is not changed and only the old parameter gc.pruneExpire is relevant. Change-Id: I6209efb05678b15153bd22479dc13486907a44f8 --- .../storage/file/GcBasicPackingTest.java | 13 ++++- .../storage/pack/GcCommitSelectionTest.java | 9 ++- .../jgit/internal/storage/file/GC.java | 58 ++++++++++++++++++- .../org/eclipse/jgit/lib/ConfigConstants.java | 6 ++ 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java index f549fb5cd..41a1a5d3f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcBasicPackingTest.java @@ -213,9 +213,20 @@ public class GcBasicPackingTest extends GcTestCase { assertEquals(9, stats.numberOfPackedObjects); assertEquals(2, stats.numberOfPackFiles); + // repack again but now without a grace period for loose objects. Since + // we don't have loose objects anymore this shouldn't change anything + gc.setExpireAgeMillis(0); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + // if objects exist in multiple packFiles then they are counted multiple + // times + assertEquals(9, stats.numberOfPackedObjects); + assertEquals(2, stats.numberOfPackFiles); + // repack again but now without a grace period for packfiles. We should // end up with one packfile - gc.setExpireAgeMillis(0); + gc.setPackExpireAgeMillis(0); gc.gc(); stats = gc.getStatistics(); assertEquals(0, stats.numberOfLooseObjects); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java index 5fda07086..49e1f6f3d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/pack/GcCommitSelectionTest.java @@ -95,7 +95,8 @@ public class GcCommitSelectionTest extends GcTestCase { } currentCommits = nextCommitCount; - gc.setExpireAgeMillis(0); // immediately delete old packs + gc.setPackExpireAgeMillis(0); // immediately delete old packs + gc.setExpireAgeMillis(0); gc.gc(); assertEquals(currentCommits * 3, // commit/tree/object gc.getStatistics().numberOfPackedObjects); @@ -158,7 +159,8 @@ public class GcCommitSelectionTest extends GcTestCase { } currentCommits = nextCommitCount; - gc.setExpireAgeMillis(0); // immediately delete old packs + gc.setPackExpireAgeMillis(0); // immediately delete old packs + gc.setExpireAgeMillis(0); gc.gc(); assertEquals(currentCommits + " commits: ", expectedBitmapCount, gc.getStatistics().numberOfBitmaps); @@ -198,7 +200,8 @@ public class GcCommitSelectionTest extends GcTestCase { final int commitsForShallowBranches = 100; // Excessive branch history pruning, one old branch. - gc.setExpireAgeMillis(0); // immediately delete old packs + gc.setPackExpireAgeMillis(0); // immediately delete old packs + gc.setExpireAgeMillis(0); gc.gc(); assertEquals( commitsForSparseBranch + commitsForFullBranch 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 49f9335ae..9cdb75322 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 @@ -111,6 +111,8 @@ import org.eclipse.jgit.util.SystemReader; public class GC { private static final String PRUNE_EXPIRE_DEFAULT = "2.weeks.ago"; //$NON-NLS-1$ + private static final String PRUNE_PACK_EXPIRE_DEFAULT = "1.hour.ago"; //$NON-NLS-1$ + private final FileRepository repo; private ProgressMonitor pm; @@ -119,6 +121,10 @@ public class GC { private Date expire; + private long packExpireAgeMillis = -1; + + private Date packExpire; + private PackConfig pconfig = null; /** @@ -186,7 +192,7 @@ public class GC { */ private void deleteOldPacks(Collection oldPacks, Collection newPacks) throws ParseException { - long expireDate = getExpireDate(); + long packExpireDate = getPackExpireDate(); oldPackLoop: for (PackFile oldPack : oldPacks) { String oldName = oldPack.getPackName(); // check whether an old pack file is also among the list of new @@ -196,7 +202,7 @@ public class GC { continue oldPackLoop; if (!oldPack.shouldBeKept() - && oldPack.getPackFile().lastModified() < expireDate) { + && oldPack.getPackFile().lastModified() < packExpireDate) { oldPack.close(); prunePack(oldName); } @@ -449,6 +455,26 @@ public class GC { return expireDate; } + private long getPackExpireDate() throws ParseException { + long packExpireDate = Long.MAX_VALUE; + + if (packExpire == null && packExpireAgeMillis == -1) { + String prunePackExpireStr = repo.getConfig().getString( + ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_PRUNEPACKEXPIRE); + if (prunePackExpireStr == null) + prunePackExpireStr = PRUNE_PACK_EXPIRE_DEFAULT; + packExpire = GitDateParser.parse(prunePackExpireStr, null, + SystemReader.getInstance().getLocale()); + packExpireAgeMillis = -1; + } + if (packExpire != null) + packExpireDate = packExpire.getTime(); + if (packExpireAgeMillis != -1) + packExpireDate = System.currentTimeMillis() - packExpireAgeMillis; + return packExpireDate; + } + /** * Remove all entries from a map which key is the id of an object referenced * by the given ObjectWalk @@ -982,6 +1008,20 @@ public class GC { expire = null; } + /** + * During gc() or prune() packfiles which are created or modified in the + * last packExpireAgeMillis milliseconds will not be deleted. + * Only older packfiles may be deleted. If set to 0 then every packfile is a + * candidate for deletion. + * + * @param packExpireAgeMillis + * minimal age of packfiles to be deleted in milliseconds. + */ + public void setPackExpireAgeMillis(long packExpireAgeMillis) { + this.packExpireAgeMillis = packExpireAgeMillis; + expire = null; + } + /** * Set the PackConfig used when (re-)writing packfiles. This allows to * influence how packs are written and to implement something similar to @@ -1011,4 +1051,18 @@ public class GC { this.expire = expire; expireAgeMillis = -1; } + + /** + * During gc() or prune() packfiles which are created or modified after or + * at packExpire will not be deleted. Only older packfiles may + * be deleted. If set to null then every packfile is a candidate for + * deletion. + * + * @param packExpire + * instant in time which defines packfile expiration + */ + public void setPackExpire(Date packExpire) { + this.packExpire = packExpire; + packExpireAgeMillis = -1; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 054d19301..3233eba00 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -241,6 +241,12 @@ public class ConfigConstants { /** The "pruneexpire" key */ public static final String CONFIG_KEY_PRUNEEXPIRE = "pruneexpire"; + /** + * The "prunepackexpire" key + * @since 4.3 + */ + public static final String CONFIG_KEY_PRUNEPACKEXPIRE = "prunepackexpire"; + /** * The "aggressiveDepth" key * @since 3.6