From 6b65adca2d50108fcb106904738b444fefe49fd1 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Sat, 13 Jun 2015 01:38:29 +0200 Subject: [PATCH] Add a grace period for packfiles during GC For loose objects an expiration date can be set which will save too young objects from being deleted. Add the same for packfiles. Packfiles which are too young are not deleted. Bug: 468024 Change-Id: I3956411d19b47aaadc215dab360d57fa6c24635e Signed-off-by: Matthias Sohn --- .../storage/file/GcBasicPackingTest.java | 44 ++++++++++++++ .../jgit/internal/storage/file/GC.java | 57 ++++++++++++------- 2 files changed, 81 insertions(+), 20 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 0742504d2..bbd41237e 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 @@ -45,13 +45,16 @@ package org.eclipse.jgit.internal.storage.file; import static org.junit.Assert.assertEquals; +import java.io.File; import java.io.IOException; import java.util.Collection; +import java.util.Date; import java.util.Iterator; import org.eclipse.jgit.junit.TestRepository.BranchBuilder; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.storage.pack.PackConfig; +import org.junit.Test; import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; @@ -176,6 +179,47 @@ public class GcBasicPackingTest extends GcTestCase { } } + @Test + public void testDonePruneTooYoungPacks() throws Exception { + BranchBuilder bb = tr.branch("refs/heads/master"); + bb.commit().message("M").add("M", "M").create(); + + gc.setExpireAgeMillis(0); + gc.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + assertEquals(3, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + File oldPackfile = tr.getRepository().getObjectDatabase().getPacks() + .iterator().next().getPackFile(); + + fsTick(); + bb.commit().message("B").add("B", "Q").create(); + + // The old packfile is too young to be deleted. We should end up with + // two pack files + gc.setExpire(new Date(oldPackfile.lastModified() - 1)); + 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.gc(); + stats = gc.getStatistics(); + assertEquals(0, stats.numberOfLooseObjects); + // if objects exist in multiple packFiles then they are counted multiple + // times + assertEquals(6, stats.numberOfPackedObjects); + assertEquals(1, stats.numberOfPackFiles); + + } + private void configureGc(GC myGc, boolean aggressive) { PackConfig pconfig = new PackConfig(repo); if (aggressive) { 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 338106f8e..c5723c059 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 @@ -175,13 +175,17 @@ public class GC { /** * Delete old pack files. What is 'old' is defined by specifying a set of * old pack files and a set of new pack files. Each pack file contained in - * old pack files but not contained in new pack files will be deleted. + * old pack files but not contained in new pack files will be deleted. If an + * expirationDate is set then pack files which are younger than the + * expirationDate will not be deleted. * * @param oldPacks * @param newPacks + * @throws ParseException */ private void deleteOldPacks(Collection oldPacks, - Collection newPacks) { + Collection newPacks) throws ParseException { + long expireDate = getExpireDate(); oldPackLoop: for (PackFile oldPack : oldPacks) { String oldName = oldPack.getPackName(); // check whether an old pack file is also among the list of new @@ -190,7 +194,8 @@ public class GC { if (oldName.equals(newPack.getPackName())) continue oldPackLoop; - if (!oldPack.shouldBeKept()) { + if (!oldPack.shouldBeKept() + && oldPack.getPackFile().lastModified() < expireDate) { oldPack.close(); prunePack(oldName); } @@ -303,22 +308,7 @@ public class GC { */ public void prune(Set objectsToKeep) throws IOException, ParseException { - long expireDate = Long.MAX_VALUE; - - if (expire == null && expireAgeMillis == -1) { - String pruneExpireStr = repo.getConfig().getString( - ConfigConstants.CONFIG_GC_SECTION, null, - ConfigConstants.CONFIG_KEY_PRUNEEXPIRE); - if (pruneExpireStr == null) - pruneExpireStr = PRUNE_EXPIRE_DEFAULT; - expire = GitDateParser.parse(pruneExpireStr, null, SystemReader - .getInstance().getLocale()); - expireAgeMillis = -1; - } - if (expire != null) - expireDate = expire.getTime(); - if (expireAgeMillis != -1) - expireDate = System.currentTimeMillis() - expireAgeMillis; + long expireDate = getExpireDate(); // Collect all loose objects which are old enough, not referenced from // the index and not in objectsToKeep @@ -435,6 +425,26 @@ public class GC { repo.getObjectDatabase().close(); } + private long getExpireDate() throws ParseException { + long expireDate = Long.MAX_VALUE; + + if (expire == null && expireAgeMillis == -1) { + String pruneExpireStr = repo.getConfig().getString( + ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_PRUNEEXPIRE); + if (pruneExpireStr == null) + pruneExpireStr = PRUNE_EXPIRE_DEFAULT; + expire = GitDateParser.parse(pruneExpireStr, null, SystemReader + .getInstance().getLocale()); + expireAgeMillis = -1; + } + if (expire != null) + expireDate = expire.getTime(); + if (expireAgeMillis != -1) + expireDate = System.currentTimeMillis() - expireAgeMillis; + return expireDate; + } + /** * Remove all entries from a map which key is the id of an object referenced * by the given ObjectWalk @@ -559,7 +569,14 @@ public class GC { if (rest != null) ret.add(rest); } - deleteOldPacks(toBeDeleted, ret); + try { + deleteOldPacks(toBeDeleted, ret); + } catch (ParseException e) { + // TODO: the exception has to be wrapped into an IOException because + // throwing the ParseException directly would break the API, instead + // we should throw a ConfigInvalidException + throw new IOException(e); + } prunePacked(); lastPackedRefs = refsBefore;