From 82b1af31e295273eeedd82fc5db3266c909c9e80 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Mon, 8 Apr 2019 13:26:12 +0100 Subject: 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 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/internal/storage/file/ObjectDirectory.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'org.eclipse.jgit/src/org') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index 44ad99bb9a..7c7a39ecc4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -127,8 +127,6 @@ public class ObjectDirectory extends FileObjectDatabase { private final File alternatesFile; - private final AtomicReference packList; - private final FS fs; private final AtomicReference alternates; @@ -141,6 +139,8 @@ public class ObjectDirectory extends FileObjectDatabase { private Set shallowCommitsIds; + final AtomicReference packList; + /** * Initialize a reference to an on-disk object directory. * @@ -674,7 +674,7 @@ public class ObjectDirectory extends FileObjectDatabase { 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 @@ public class ObjectDirectory extends FileObjectDatabase { 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 class ObjectDirectory extends FileObjectDatabase { 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; -- cgit v1.2.3 From d6e00d201514ce69addcbfb1c7163f42336f3e66 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 9 Apr 2019 10:44:06 +0100 Subject: Remember the cause for invalidating a packfile Keep track of the original cause for a packfile invalidation. It is needed for the sysadmin to understand if there is a real underlying filesystem problem and repository corruption or if it is simply a consequence of a concurrency of Git operations (e.g. repack or GC). Change-Id: I06ddda9ec847844ec31616ab6d17f153a5a34e33 Signed-off-by: Luca Milanesio Signed-off-by: David Pursehouse Signed-off-by: Matthias Sohn --- org.eclipse.jgit/.settings/.api_filters | 14 +++++++++ .../eclipse/jgit/errors/PackInvalidException.java | 34 ++++++++++++++++++++-- .../jgit/internal/storage/dfs/DfsPackFile.java | 19 ++++++++---- .../internal/storage/file/ObjectDirectory.java | 9 ++---- .../jgit/internal/storage/file/PackFile.java | 23 +++++++++------ 5 files changed, 76 insertions(+), 23 deletions(-) (limited to 'org.eclipse.jgit/src/org') diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 0b5acc2bd0..eb31d56ed1 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -8,6 +8,20 @@ + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/errors/PackInvalidException.java b/org.eclipse.jgit/src/org/eclipse/jgit/errors/PackInvalidException.java index 8c216c3d41..09be9ec349 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/errors/PackInvalidException.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/errors/PackInvalidException.java @@ -58,9 +58,24 @@ public class PackInvalidException extends IOException { * * @param path * path of the invalid pack file. + * @deprecated Use {@link #PackInvalidException(File, Throwable)}. */ + @Deprecated public PackInvalidException(final File path) { - this(path.getAbsolutePath()); + this(path, null); + } + + /** + * Construct a pack invalid error with cause. + * + * @param path + * path of the invalid pack file. + * @param cause + * cause of the pack file becoming invalid. + * @since 4.5.7 + */ + public PackInvalidException(final File path, Throwable cause) { + this(path.getAbsolutePath(), cause); } /** @@ -68,8 +83,23 @@ public class PackInvalidException extends IOException { * * @param path * path of the invalid pack file. + * @deprecated Use {@link #PackInvalidException(String, Throwable)}. */ + @Deprecated public PackInvalidException(final String path) { - super(MessageFormat.format(JGitText.get().packFileInvalid, path)); + this(path, null); + } + + /** + * Construct a pack invalid error with cause. + * + * @param path + * path of the invalid pack file. + * @param cause + * cause of the pack file becoming invalid. + * @since 4.5.7 + */ + public PackInvalidException(final String path, Throwable cause) { + super(MessageFormat.format(JGitText.get().packFileInvalid, path), cause); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java index 4eabb03163..f845453ec4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackFile.java @@ -135,6 +135,9 @@ public final class DfsPackFile { /** True once corruption has been detected that cannot be worked around. */ private volatile boolean invalid; + /** Exception that caused the packfile to be flagged as invalid */ + private volatile Exception invalidatingCause; + /** * Lock for initialization of {@link #index} and {@link #corruptObjects}. *

@@ -236,8 +239,9 @@ public final class DfsPackFile { return idx; } - if (invalid) - throw new PackInvalidException(getPackName()); + if (invalid) { + throw new PackInvalidException(getPackName(), invalidatingCause); + } Repository.getGlobalListenerList() .dispatch(new BeforeDfsPackIndexLoadedEvent(this)); @@ -268,6 +272,7 @@ public final class DfsPackFile { } } catch (EOFException e) { invalid = true; + invalidatingCause = e; IOException e2 = new IOException(MessageFormat.format( DfsText.get().shortReadOfIndex, packDesc.getFileName(INDEX))); @@ -275,6 +280,7 @@ public final class DfsPackFile { throw e2; } catch (IOException e) { invalid = true; + invalidatingCause = e; IOException e2 = new IOException(MessageFormat.format( DfsText.get().cannotReadIndex, packDesc.getFileName(INDEX))); @@ -743,8 +749,10 @@ public final class DfsPackFile { private IOException packfileIsTruncated() { invalid = true; - return new IOException(MessageFormat.format( + IOException exc = new IOException(MessageFormat.format( JGitText.get().packfileIsTruncated, getPackName())); + invalidatingCause = exc; + return exc; } private void readFully(long position, byte[] dstbuf, int dstoff, int cnt, @@ -766,8 +774,9 @@ public final class DfsPackFile { DfsBlock readOneBlock(long pos, DfsReader ctx) throws IOException { - if (invalid) - throw new PackInvalidException(getPackName()); + if (invalid) { + throw new PackInvalidException(getPackName(), invalidatingCause); + } ReadableChannel rc = ctx.db.openFile(packDesc, PACK); try { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index 7c7a39ecc4..019d3ed3e9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -581,13 +581,8 @@ public class ObjectDirectory extends FileObjectDatabase { transientErrorCount = p.incrementTransientErrorCount(); } if (warnTmpl != null) { - if (LOG.isDebugEnabled()) { - LOG.debug(MessageFormat.format(warnTmpl, - p.getPackFile().getAbsolutePath()), e); - } else { - LOG.warn(MessageFormat.format(warnTmpl, - p.getPackFile().getAbsolutePath())); - } + LOG.warn(MessageFormat.format(warnTmpl, + p.getPackFile().getAbsolutePath()), e); } else { if (doLogExponentialBackoff(transientErrorCount)) { // Don't remove the pack from the list, as the error may be diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index 7a51a5a02f..f49e57def2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -133,6 +133,8 @@ public class PackFile implements Iterable { private volatile boolean invalid; + private volatile Exception invalidatingCause; + private boolean invalidBitmap; private AtomicInteger transientErrorCount = new AtomicInteger(); @@ -177,8 +179,9 @@ public class PackFile implements Iterable { private synchronized PackIndex idx() throws IOException { if (loadedIdx == null) { - if (invalid) - throw new PackInvalidException(packFile); + if (invalid) { + throw new PackInvalidException(packFile, invalidatingCause); + } try { final PackIndex idx = PackIndex.open(extFile(INDEX)); @@ -196,6 +199,7 @@ public class PackFile implements Iterable { throw e; } catch (IOException e) { invalid = true; + invalidatingCause = e; throw e; } } @@ -644,7 +648,7 @@ public class PackFile implements Iterable { private void doOpen() throws IOException { if (invalid) { - throw new PackInvalidException(packFile); + throw new PackInvalidException(packFile, invalidatingCause); } try { synchronized (readLock) { @@ -654,13 +658,13 @@ public class PackFile implements Iterable { } } catch (InterruptedIOException e) { // don't invalidate the pack, we are interrupted from another thread - openFail(false); + openFail(false, e); throw e; } catch (FileNotFoundException fn) { // don't invalidate the pack if opening an existing file failed // since it may be related to a temporary lack of resources (e.g. // max open files) - openFail(!packFile.exists()); + openFail(!packFile.exists(), fn); throw fn; } catch (EOFException | AccessDeniedException | NoSuchFileException | CorruptObjectException | NoPackSignatureException @@ -668,20 +672,21 @@ public class PackFile implements Iterable { | UnsupportedPackIndexVersionException | UnsupportedPackVersionException pe) { // exceptions signaling permanent problems with a pack - openFail(true); + openFail(true, pe); throw pe; } catch (IOException | RuntimeException ge) { // generic exceptions could be transient so we should not mark the // pack invalid to avoid false MissingObjectExceptions - openFail(false); + openFail(false, ge); throw ge; } } - private void openFail(boolean invalidate) { + private void openFail(boolean invalidate, Exception cause) { activeWindows = 0; activeCopyRawData = 0; invalid = invalidate; + invalidatingCause = cause; doClose(); } @@ -708,7 +713,7 @@ public class PackFile implements Iterable { // Detect the situation and throw a proper exception so that can be properly // managed by the main packfile search loop and the Git client won't receive // any failures. - throw new PackInvalidException(packFile); + throw new PackInvalidException(packFile, invalidatingCause); } if (length < pos + size) size = (int) (length - pos); -- cgit v1.2.3