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 <matthias.sohn@sap.com>

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) {

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<PackFile> oldPacks,
-			Collection<PackFile> newPacks) {
+			Collection<PackFile> 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<ObjectId> 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;