Don't delete .idx file if .pack file can't be deleted
If during an garbage collection old packfiles are deleted it could happen that on certain platforms the index file can be deleted but the packfile can't be deleted (because someone locked the file). This led to repositories with packfiles without corresponding index files. Those zombie-packfiles potentially consume a lot of space on disk and it is never tried to delete them again. Try to avoid this situation by deleting packfiles first and don't try to delete the other files if we can't delete the packfile. This gives us the chance to delete the packfile during next GC. This commit only improves the situation - there is still the chance for orphan files during packfile deletion. We don't have an atomic delete of multiple files . Change-Id: I0a19ae630186f07d0cc7fe9df246fa1cedeca8f6
This commit is contained in:
parent
f1c086f692
commit
8da939221c
|
@ -49,6 +49,7 @@
|
|||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
import java.io.IOException;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
|
@ -66,6 +67,7 @@
|
|||
import org.eclipse.jgit.internal.JGitText;
|
||||
import org.eclipse.jgit.internal.storage.file.GC.RepoStatistics;
|
||||
import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
|
||||
import org.eclipse.jgit.internal.storage.pack.PackExt;
|
||||
import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
|
||||
import org.eclipse.jgit.junit.RepositoryTestCase;
|
||||
import org.eclipse.jgit.junit.TestRepository;
|
||||
|
@ -620,6 +622,40 @@ public void testPackCommitsAndLooseOneWithPruneNow() throws Exception {
|
|||
assertEquals(2, stats.numberOfPackFiles);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPruneOldPacksWithOpenHandleOnPack() throws Exception {
|
||||
gc.setExpireAgeMillis(0);
|
||||
|
||||
BranchBuilder bb = tr.branch("refs/heads/master");
|
||||
bb.commit().add("A", "A").add("B", "B").create();
|
||||
fsTick();
|
||||
gc.gc();
|
||||
|
||||
Collection<PackFile> packs = repo.getObjectDatabase().getPacks();
|
||||
assertEquals(1, packs.size());
|
||||
PackFile pack = packs.iterator().next();
|
||||
File packFile = pack.getPackFile();
|
||||
File indexFile = new File(packFile.getParentFile(), "pack-"
|
||||
+ pack.getPackName()
|
||||
+ "."
|
||||
+ PackExt.INDEX.getExtension());
|
||||
FileInputStream fis = new FileInputStream(packFile);
|
||||
try {
|
||||
bb.commit().add("A", "A2").add("B", "B2").create();
|
||||
fsTick();
|
||||
gc.gc();
|
||||
if (packFile.exists()) {
|
||||
assertTrue(
|
||||
"The pack was present but the index file was missing.",
|
||||
indexFile.exists());
|
||||
}
|
||||
|
||||
} finally {
|
||||
fis.close();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPackCommitsAndLooseOneWithPruneNowNoReflog()
|
||||
throws Exception {
|
||||
|
|
|
@ -175,21 +175,9 @@ public Collection<PackFile> gc() throws IOException, ParseException {
|
|||
*
|
||||
* @param oldPacks
|
||||
* @param newPacks
|
||||
* @param ignoreErrors
|
||||
* <code>true</code> if we should ignore the fact that a certain
|
||||
* pack files or index files couldn't be deleted.
|
||||
* <code>false</code> if an exception should be thrown in such
|
||||
* cases
|
||||
* @throws IOException
|
||||
* if a pack file couldn't be deleted and
|
||||
* <code>ignoreErrors</code> is set to <code>false</code>
|
||||
*/
|
||||
private void deleteOldPacks(Collection<PackFile> oldPacks,
|
||||
Collection<PackFile> newPacks, boolean ignoreErrors)
|
||||
throws IOException {
|
||||
int deleteOptions = FileUtils.RETRY | FileUtils.SKIP_MISSING;
|
||||
if (ignoreErrors)
|
||||
deleteOptions |= FileUtils.IGNORE_ERRORS;
|
||||
Collection<PackFile> newPacks) {
|
||||
oldPackLoop: for (PackFile oldPack : oldPacks) {
|
||||
String oldName = oldPack.getPackName();
|
||||
// check whether an old pack file is also among the list of new
|
||||
|
@ -200,10 +188,7 @@ private void deleteOldPacks(Collection<PackFile> oldPacks,
|
|||
|
||||
if (!oldPack.shouldBeKept()) {
|
||||
oldPack.close();
|
||||
for (PackExt ext : PackExt.values()) {
|
||||
File f = nameFor(oldName, "." + ext.getExtension()); //$NON-NLS-1$
|
||||
FileUtils.delete(f, deleteOptions);
|
||||
}
|
||||
prunePack(oldName);
|
||||
}
|
||||
}
|
||||
// close the complete object database. Thats my only chance to force
|
||||
|
@ -211,6 +196,42 @@ private void deleteOldPacks(Collection<PackFile> oldPacks,
|
|||
repo.getObjectDatabase().close();
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete files associated with a single pack file. First try to delete the
|
||||
* ".pack" file because on some platforms the ".pack" file may be locked and
|
||||
* can't be deleted. In such a case it is better to detect this early and
|
||||
* give up on deleting files for this packfile. Otherwise we may delete the
|
||||
* ".index" file and when failing to delete the ".pack" file we are left
|
||||
* with a ".pack" file without a ".index" file.
|
||||
*
|
||||
* @param packName
|
||||
*/
|
||||
private void prunePack(String packName) {
|
||||
PackExt[] extensions = PackExt.values();
|
||||
try {
|
||||
// Delete the .pack file first and if this fails give up on deleting
|
||||
// the other files
|
||||
int deleteOptions = FileUtils.RETRY | FileUtils.SKIP_MISSING;
|
||||
for (PackExt ext : extensions)
|
||||
if (PackExt.PACK.equals(ext)) {
|
||||
File f = nameFor(packName, "." + ext.getExtension()); //$NON-NLS-1$
|
||||
FileUtils.delete(f, deleteOptions);
|
||||
break;
|
||||
}
|
||||
// The .pack file has been deleted. Delete as many as the other
|
||||
// files as you can.
|
||||
deleteOptions |= FileUtils.IGNORE_ERRORS;
|
||||
for (PackExt ext : extensions) {
|
||||
if (!PackExt.PACK.equals(ext)) {
|
||||
File f = nameFor(packName, "." + ext.getExtension()); //$NON-NLS-1$
|
||||
FileUtils.delete(f, deleteOptions);
|
||||
}
|
||||
}
|
||||
} catch (IOException e) {
|
||||
// Deletion of the .pack file failed. Silently return.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Like "git prune-packed" this method tries to prune all loose objects
|
||||
* which can be found in packs. If certain objects can't be pruned (e.g.
|
||||
|
@ -533,7 +554,7 @@ public Collection<PackFile> repack() throws IOException {
|
|||
if (rest != null)
|
||||
ret.add(rest);
|
||||
}
|
||||
deleteOldPacks(toBeDeleted, ret, true);
|
||||
deleteOldPacks(toBeDeleted, ret);
|
||||
prunePacked();
|
||||
|
||||
lastPackedRefs = refsBefore;
|
||||
|
|
Loading…
Reference in New Issue