From 4c558225dc991a34d59bc4fb0d4e6f1f6974f53c Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 10 Mar 2017 00:20:23 +0000 Subject: Don't remove pack when FileNotFoundException is transient The FileNotFoundException is typically raised in three conditions: 1. file doesn't exist 2. incompatible read vs. read/write open modes 3. filesystem locking 4. temporary lack of resources (e.g. too many open files) 1. is already managed, 2. would never happen as packs are not overwritten while with 3. and 4. it is worth logging the exception and retrying to read the pack again. Log transient errors using an exponential backoff strategy to avoid flooding the logs with the same error if consecutive retries to access the pack fail repeatedly. Bug: 513435 Change-Id: I03c6f6891de3c343d3d517092eaa75dba282c0cd Signed-off-by: Luca Milanesio Signed-off-by: Matthias Sohn --- .../internal/storage/file/ObjectDirectory.java | 34 +++++++++++++++++----- .../jgit/internal/storage/file/PackFile.java | 11 +++++++ 2 files changed, 38 insertions(+), 7 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse/jgit') 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 ea80528518..6489415ebb 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 @@ -337,6 +337,7 @@ public class ObjectDirectory extends FileObjectDatabase { for (PackFile p : pList.packs) { try { p.resolve(matches, id, RESOLVE_ABBREV_LIMIT); + p.resetTransientErrorCount(); } catch (IOException e) { handlePackError(e, p); } @@ -418,6 +419,7 @@ public class ObjectDirectory extends FileObjectDatabase { for (PackFile p : pList.packs) { try { ObjectLoader ldr = p.get(curs, objectId); + p.resetTransientErrorCount(); if (ldr != null) return ldr; } catch (PackMismatchException e) { @@ -496,6 +498,7 @@ public class ObjectDirectory extends FileObjectDatabase { for (PackFile p : pList.packs) { try { long len = p.getObjectSize(curs, id); + p.resetTransientErrorCount(); if (0 <= len) return len; } catch (PackMismatchException e) { @@ -535,6 +538,7 @@ public class ObjectDirectory extends FileObjectDatabase { for (final PackFile p : pList.packs) { try { LocalObjectRepresentation rep = p.representation(curs, otp); + p.resetTransientErrorCount(); if (rep != null) packer.select(otp, rep); } catch (PackMismatchException e) { @@ -555,6 +559,8 @@ public class ObjectDirectory extends FileObjectDatabase { private void handlePackError(IOException e, PackFile p) { String warnTmpl = null; + int transientErrorCount = 0; + String errTmpl = JGitText.get().exceptionWhileReadingPack; if ((e instanceof CorruptObjectException) || (e instanceof PackInvalidException)) { warnTmpl = JGitText.get().corruptPack; @@ -562,14 +568,17 @@ public class ObjectDirectory extends FileObjectDatabase { removePack(p); } else if (e instanceof FileNotFoundException) { if (p.getPackFile().exists()) { - warnTmpl = JGitText.get().packInaccessible; + errTmpl = JGitText.get().packInaccessible; + transientErrorCount = p.incrementTransientErrorCount(); } else { warnTmpl = JGitText.get().packWasDeleted; + removePack(p); } - removePack(p); } else if (FileUtils.isStaleFileHandle(e)) { warnTmpl = JGitText.get().packHandleIsStale; removePack(p); + } else { + transientErrorCount = p.incrementTransientErrorCount(); } if (warnTmpl != null) { if (LOG.isDebugEnabled()) { @@ -580,14 +589,25 @@ public class ObjectDirectory extends FileObjectDatabase { p.getPackFile().getAbsolutePath())); } } else { - // Don't remove the pack from the list, as the error may be - // transient. - LOG.error(MessageFormat.format( - JGitText.get().exceptionWhileReadingPack, p.getPackFile() - .getAbsolutePath()), e); + if (doLogExponentialBackoff(transientErrorCount)) { + // Don't remove the pack from the list, as the error may be + // transient. + LOG.error(MessageFormat.format(errTmpl, + p.getPackFile().getAbsolutePath()), + Integer.valueOf(transientErrorCount), e); + } } } + /** + * @param n + * count of consecutive failures + * @return @{code true} if i is a power of 2 + */ + private boolean doLogExponentialBackoff(int n) { + return (n & (n - 1)) == 0; + } + @Override InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id, boolean createDuplicate) throws IOException { 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 b385b8ab73..e004f90902 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 @@ -61,6 +61,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.Iterator; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.zip.CRC32; import java.util.zip.DataFormatException; import java.util.zip.Inflater; @@ -125,6 +126,8 @@ public class PackFile implements Iterable { private boolean invalidBitmap; + private AtomicInteger transientErrorCount = new AtomicInteger(); + private byte[] packChecksum; private PackIndex loadedIdx; @@ -568,6 +571,14 @@ public class PackFile implements Iterable { invalid = true; } + int incrementTransientErrorCount() { + return transientErrorCount.incrementAndGet(); + } + + void resetTransientErrorCount() { + transientErrorCount.set(0); + } + private void readFully(final long position, final byte[] dstbuf, int dstoff, final int cnt, final WindowCursor curs) throws IOException { -- cgit v1.2.3