motiejus
/
jgit
1
Fork 0
Code Releases Activity

Delete expired garbage even when there is no GC pack present. 

Delete the condition to check whether the garbage pack creation time
is older than the last GC operation, because it's not possible to
find the last GC operation time when there is no GC pack.

Add additional tests to make sure the contents of the expired garbage
packs are considered during the GC operation and any actively
referenced objects from the garbage packs are copied successfully
into the GC pack before deleting the garbage pack.

Change-Id: I09e8b2656de8ba7f9b996724ad1961d908e937b6
Signed-off-by: Thirumala Reddy Mutchukota <thirumala@google.com>
This commit is contained in:
Thirumala Reddy Mutchukota 2017-04-21 12:26:33 -07:00
parent 6a4b821268
commit 5e250e45be
2 changed files with 119 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

141
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java
View File

@ -7,7 +7,6 @@
import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
@ -180,41 +179,133 @@ public void testCollectionWithGarbageAndGarbagePacksPurged()
RevCommit commit1 = commit().message("1").parent(commit0).create();
git.update("master", commit0);
gcNoTtl();
gcWithTtl();
// The repository has an UNREACHABLE_GARBAGE pack that could have
// expired, but since we never purge the most recent UNREACHABLE_GARBAGE
// pack, it must have survived the GC.
boolean commit1Found = false;
// The repository should have a GC pack with commit0 and an
// UNREACHABLE_GARBAGE pack with commit1.
assertEquals(2, odb.getPacks().length);
boolean gcPackFound = false;
boolean garbagePackFound = false;
for (DfsPackFile pack : odb.getPacks()) {
DfsPackDescription d = pack.getPackDescription();
if (d.getPackSource() == GC) {
gcPackFound = true;
assertTrue("has commit0", isObjectInPack(commit0, pack));
assertFalse("no commit1", isObjectInPack(commit1, pack));
} else if (d.getPackSource() == UNREACHABLE_GARBAGE) {
commit1Found |= isObjectInPack(commit1, pack);
garbagePackFound = true;
assertFalse("no commit0", isObjectInPack(commit0, pack));
assertTrue("has commit1", isObjectInPack(commit1, pack));
} else {
fail("unexpected " + d.getPackSource());
}
}
assertTrue("garbage commit1 still readable", commit1Found);
// Find oldest UNREACHABLE_GARBAGE; it will be pruned by next GC.
DfsPackDescription oldestGarbagePack = null;
for (DfsPackFile pack : odb.getPacks()) {
DfsPackDescription d = pack.getPackDescription();
if (d.getPackSource() == UNREACHABLE_GARBAGE) {
oldestGarbagePack = oldestPack(oldestGarbagePack, d);
}
}
assertNotNull("has UNREACHABLE_GARBAGE", oldestGarbagePack);
assertTrue("gc pack found", gcPackFound);
assertTrue("garbage pack found", garbagePackFound);
gcWithTtl();
assertTrue("has packs", odb.getPacks().length > 0);
// The gc operation should have removed UNREACHABLE_GARBAGE pack along with commit1.
DfsPackFile[] packs = odb.getPacks();
assertEquals(1, packs.length);
assertEquals(GC, packs[0].getPackDescription().getPackSource());
assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
assertFalse("no commit1", isObjectInPack(commit1, packs[0]));
}
@Test
public void testCollectionWithGarbageAndRereferencingGarbage()
throws Exception {
RevCommit commit0 = commit().message("0").create();
RevCommit commit1 = commit().message("1").parent(commit0).create();
git.update("master", commit0);
gcWithTtl();
// The repository should have a GC pack with commit0 and an
// UNREACHABLE_GARBAGE pack with commit1.
assertEquals(2, odb.getPacks().length);
boolean gcPackFound = false;
boolean garbagePackFound = false;
for (DfsPackFile pack : odb.getPacks()) {
assertNotEquals(oldestGarbagePack, pack.getPackDescription());
DfsPackDescription d = pack.getPackDescription();
if (d.getPackSource() == GC) {
gcPackFound = true;
assertTrue("has commit0", isObjectInPack(commit0, pack));
assertFalse("no commit1", isObjectInPack(commit1, pack));
} else if (d.getPackSource() == UNREACHABLE_GARBAGE) {
garbagePackFound = true;
assertFalse("no commit0", isObjectInPack(commit0, pack));
assertTrue("has commit1", isObjectInPack(commit1, pack));
} else {
fail("unexpected " + d.getPackSource());
}
}
assertTrue("gc pack found", gcPackFound);
assertTrue("garbage pack found", garbagePackFound);
git.update("master", commit1);
gcWithTtl();
// The gc operation should have removed the UNREACHABLE_GARBAGE pack and
// moved commit1 into GC pack.
DfsPackFile[] packs = odb.getPacks();
assertEquals(1, packs.length);
assertEquals(GC, packs[0].getPackDescription().getPackSource());
assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
}
@Test
public void testCollectionWithPureGarbageAndGarbagePacksPurged()
throws Exception {
RevCommit commit0 = commit().message("0").create();
RevCommit commit1 = commit().message("1").parent(commit0).create();
gcWithTtl();
// The repository should have a single UNREACHABLE_GARBAGE pack with commit0
// and commit1.
DfsPackFile[] packs = odb.getPacks();
assertEquals(1, packs.length);
assertEquals(UNREACHABLE_GARBAGE, packs[0].getPackDescription().getPackSource());
assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
gcWithTtl();
// The gc operation should have removed UNREACHABLE_GARBAGE pack along
// with commit0 and commit1.
assertEquals(0, odb.getPacks().length);
}
@Test
public void testCollectionWithPureGarbageAndRereferencingGarbage()
throws Exception {
RevCommit commit0 = commit().message("0").create();
RevCommit commit1 = commit().message("1").parent(commit0).create();
gcWithTtl();
// The repository should have a single UNREACHABLE_GARBAGE pack with commit0
// and commit1.
DfsPackFile[] packs = odb.getPacks();
assertEquals(1, packs.length);
DfsPackDescription pack = packs[0].getPackDescription();
assertEquals(UNREACHABLE_GARBAGE, pack.getPackSource());
assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
assertTrue("has commit1", isObjectInPack(commit1, packs[0]));
git.update("master", commit0);
gcWithTtl();
// The gc operation should have moved commit0 into the GC pack and
// removed UNREACHABLE_GARBAGE along with commit1.
packs = odb.getPacks();
assertEquals(1, packs.length);
pack = packs[0].getPackDescription();
assertEquals(GC, pack.getPackSource());
assertTrue("has commit0", isObjectInPack(commit0, packs[0]));
assertFalse("no commit1", isObjectInPack(commit1, packs[0]));
}
@Test
@ -588,14 +679,6 @@ private boolean isObjectInPack(AnyObjectId id, DfsPackFile pack)
}
}
private static DfsPackDescription oldestPack(DfsPackDescription a,
DfsPackDescription b) {
if (a != null && a.getLastModified() < b.getLastModified()) {
return a;
}
return b;
}
private int countPacks(PackSource source) throws IOException {
int cnt = 0;
for (DfsPackFile pack : odb.getPacks()) {

53
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java
View File

@ -58,7 +58,6 @@
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.GregorianCalendar;
import java.util.HashSet;
@ -242,13 +241,6 @@ public boolean pack(ProgressMonitor pm) throws IOException {
Collection<Ref> refsBefore = getAllRefs();
readPacksBefore();
if (packsBefore.isEmpty()) {
if (!expiredGarbagePacks.isEmpty()) {
objdb.commitPack(noPacks(), toPrune());
}
return true;
}
allHeads = new HashSet<>();
nonHeads = new HashSet<>();
txnHeads = new HashSet<>();
@ -307,13 +299,12 @@ private void readPacksBefore() throws IOException {
packsBefore = new ArrayList<>(packs.length);
expiredGarbagePacks = new ArrayList<>(packs.length);
long mostRecentGC = mostRecentGC(packs);
long now = SystemReader.getInstance().getCurrentTime();
for (DfsPackFile p : packs) {
DfsPackDescription d = p.getPackDescription();
if (d.getPackSource() != UNREACHABLE_GARBAGE) {
packsBefore.add(p);
} else if (packIsExpiredGarbage(d, mostRecentGC, now)) {
} else if (packIsExpiredGarbage(d, now)) {
expiredGarbagePacks.add(p);
} else if (packIsCoalesceableGarbage(d, now)) {
packsBefore.add(p);
@ -321,39 +312,13 @@ private void readPacksBefore() throws IOException {
}
}
private static long mostRecentGC(DfsPackFile[] packs) {
long r = 0;
for (DfsPackFile p : packs) {
DfsPackDescription d = p.getPackDescription();
if (d.getPackSource() == GC || d.getPackSource() == GC_REST) {
r = Math.max(r, d.getLastModified());
}
}
return r;
}
private boolean packIsExpiredGarbage(DfsPackDescription d,
long mostRecentGC, long now) {
// It should be safe to remove an UNREACHABLE_GARBAGE pack if it:
//
// (a) Predates the most recent prior run of this class. This check
// ensures the graph traversal algorithm had a chance to consider
// all objects in this pack and copied them into a GC or GC_REST
// pack if the graph contained live edges to the objects.
//
// This check is safe because of the ordering of packing; the GC
// packs are written first and then the UNREACHABLE_GARBAGE is
// constructed. Any UNREACHABLE_GARBAGE dated earlier than the GC
// was input to the prior GC's graph traversal.
//
// (b) Is older than garbagePackTtl. This check gives concurrent
// inserter threads sufficient time to identify an object is not
// in the graph and should have a new copy written, rather than
// relying on something from an UNREACHABLE_GARBAGE pack.
//
// Both (a) and (b) must be met to safely remove UNREACHABLE_GARBAGE.
private boolean packIsExpiredGarbage(DfsPackDescription d, long now) {
// Consider the garbage pack as expired when it's older than
// garbagePackTtl. This check gives concurrent inserter threads
// sufficient time to identify an object is not in the graph and should
// have a new copy written, rather than relying on something from an
// UNREACHABLE_GARBAGE pack.
return d.getPackSource() == UNREACHABLE_GARBAGE
&& d.getLastModified() < mostRecentGC
&& garbageTtlMillis > 0
&& now - d.getLastModified() >= garbageTtlMillis;
}
@ -605,8 +570,4 @@ private DfsPackDescription writePack(PackSource source, PackWriter pw,
DfsBlockCache.getInstance().getOrCreate(pack, null);
return pack;
}
private static List<DfsPackDescription> noPacks() {
return Collections.emptyList();
}
}