Fix pack files scan when filesnapshot isn't modified

Do not reload packfiles when their associated filesnapshot is not
modified on disk compared to the one currently stored in memory.

Fix the regression introduced by fef78212 which, in conjunction with
core.trustfolderstats = false, caused any lookup of objects inside
the packlist to loop forever when the object was not found in the pack
list.

Bug: 546190
Change-Id: I38d752ebe47cefc3299740aeba319a2641f19391
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This commit is contained in:
Luca Milanesio 2019-04-08 13:26:12 +01:00
parent a47367e5fb
commit 82b1af31e2
2 changed files with 54 additions and 6 deletions

View File

@ -42,6 +42,10 @@
package org.eclipse.jgit.internal.storage.file;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
@ -49,14 +53,29 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.eclipse.jgit.internal.storage.file.ObjectDirectory;
import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
public class ObjectDirectoryTest extends RepositoryTestCase {
@Parameter
public Boolean trustFolderStats;
@Parameters(name= "core.trustfolderstat={0}")
public static Iterable<? extends Object> data() {
return Arrays.asList(Boolean.TRUE, Boolean.FALSE);
}
@Test
public void testConcurrentInsertionOfBlobsToTheSameNewFanOutDirectory()
throws Exception {
@ -69,6 +88,34 @@ public void testConcurrentInsertionOfBlobsToTheSameNewFanOutDirectory()
}
}
@Test
public void testShouldNotSearchPacksAgainTheSecondTime() throws Exception {
FileRepository bareRepository = newTestRepositoryWithOnePackfile();
ObjectDirectory dir = bareRepository.getObjectDatabase();
// Make sure that timestamps are modified and read so that a full
// file snapshot check is performed
Thread.sleep(3000L);
assertTrue(dir.searchPacksAgain(dir.packList.get()));
assertFalse(dir.searchPacksAgain(dir.packList.get()));
}
private FileRepository newTestRepositoryWithOnePackfile() throws Exception {
FileRepository repository = createBareRepository();
TestRepository<FileRepository> testRepository = new TestRepository<FileRepository>(repository);
testRepository.commit();
testRepository.packAndPrune();
FileBasedConfig repoConfig = repository.getConfig();
repoConfig.setBoolean(ConfigConstants.CONFIG_CORE_SECTION,null,
ConfigConstants.CONFIG_KEY_TRUSTFOLDERSTAT,
trustFolderStats.booleanValue());
repoConfig.save();
return repository;
}
private Collection<Callable<ObjectId>> blobInsertersForTheSameFanOutDir(
final ObjectDirectory dir) {
Callable<ObjectId> callable = new Callable<ObjectId>() {

View File

@ -127,8 +127,6 @@ public class ObjectDirectory extends FileObjectDatabase {
private final File alternatesFile;
private final AtomicReference<PackList> packList;
private final FS fs;
private final AtomicReference<AlternateHandle[]> alternates;
@ -141,6 +139,8 @@ public class ObjectDirectory extends FileObjectDatabase {
private Set<ObjectId> shallowCommitsIds;
final AtomicReference<PackList> packList;
/**
* Initialize a reference to an on-disk object directory.
*
@ -674,7 +674,7 @@ InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id,
return InsertLooseObjectResult.FAILURE;
}
private boolean searchPacksAgain(PackList old) {
boolean searchPacksAgain(PackList old) {
// Whether to trust the pack folder's modification time. If set
// to false we will always scan the .git/objects/pack folder to
// check for new pack files. If set to true (default) we use the
@ -822,7 +822,8 @@ private PackList scanPacksImpl(final PackList old) {
final String packName = base + PACK.getExtension();
final File packFile = new File(packDirectory, packName);
final PackFile oldPack = forReuse.remove(packName);
if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) {
if (oldPack != null
&& !oldPack.getFileSnapshot().isModified(packFile)) {
list.add(oldPack);
continue;
}
@ -960,7 +961,7 @@ public File fileFor(AnyObjectId objectId) {
return new File(new File(getDirectory(), d), f);
}
private static final class PackList {
static final class PackList {
/** State just before reading the pack directory. */
final FileSnapshot snapshot;