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 <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 327ca0a10bdfdf6f05dbc809ecbc882039aa1216..b5b04b4743921536f3131d4ae7a89a96df7f067e 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -268,7 +268,7 @@ exceptionCaughtDuringExcecutionOfCommand=Exception caught during execution of co
 exceptionHookExecutionInterrupted=Execution of "{0}" hook interrupted.
 exceptionOccurredDuringAddingOfOptionToALogCommand=Exception occurred during adding of {0} as option to a Log command
 exceptionOccurredDuringReadingOfGIT_DIR=Exception occurred during reading of $GIT_DIR/{0}. {1}
-exceptionWhileReadingPack=ERROR: Exception caught while accessing pack file {0}, the pack file might be corrupt
+exceptionWhileReadingPack=ERROR: Exception caught while accessing pack file {0}, the pack file might be corrupt, {1}. Caught {2} consecutive errors while trying to read this pack.
 expectedACKNAKFoundEOF=Expected ACK/NAK, found EOF
 expectedACKNAKGot=Expected ACK/NAK, got: {0}
 expectedBooleanStringValue=Expected boolean string value
@@ -469,7 +469,7 @@ packfileIsTruncated=Packfile {0} is truncated.
 packfileIsTruncatedNoParam=Packfile is truncated.
 packHandleIsStale=Pack file {0} handle is stale, removing it from pack list
 packHasUnresolvedDeltas=pack has unresolved deltas
-packInaccessible=Pack file {0} now inaccessible; removing it from pack list
+packInaccessible=Failed to access pack file {0},  caught {2} consecutive errors while trying to access this pack.
 packingCancelledDuringObjectsWriting=Packing cancelled during objects writing
 packObjectCountMismatch=Pack object count mismatch: pack {0} index {1}: {2}
 packRefs=Pack refs

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 ea80528518f5e94a7b4d0d88bfc5a45105a86d9d..6489415ebbb3082d1f49ff0bd8db835a9df715c0 100644 (file)
--- 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 b385b8ab735646a7e7bb19af5b67e53794ad51ed..e004f90902e13af768fd6ee987511b6acf0b330c 100644 (file)
--- 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<PackIndex.MutableEntry> {
 
        private boolean invalidBitmap;
 
+       private AtomicInteger transientErrorCount = new AtomicInteger();
+
        private byte[] packChecksum;
 
        private PackIndex loadedIdx;
@@ -568,6 +571,14 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
                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 {