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>

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 0742504d23a7b940d2246a33e8312fb984bf5c65..bbd41237e23fe4b161548a26a1584975819a5602 100644 (file)
--- 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 338106f8e78c868ae1c6b752d58f8779f65be642..c5723c05940635386b0e8101f7a2efedfb8fba99 100644 (file)
--- 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<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;