Don't remove pack from pack list for problems which could be transient

If we hit a corrupt object or invalid pack remove the pack from the pack
list. Other IOException could be transient hence we should not remove
the pack from the list to avoid the problem reported on the Gerrit list
[1]. It looks like in the reported case the pack was removed from the
pack list causing MissingObjectExceptions which disappear when the
server is restarted.

[1] https://groups.google.com/forum/#!topic/repo-discuss/Qdmbl-YZ4NU

Change-Id: I331626110d54b190e46cddc2c40f29ddeb9613cd
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

3 files changed, 21 insertions, 15 deletions

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 b0ce1582d4..55cf3ea71e 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -145,6 +145,7 @@ couldNotRenameTemporaryIndexFileToIndex=Could not rename temporary index file to
 couldNotURLEncodeToUTF8=Could not URL encode to UTF-8
 couldNotWriteFile=Could not write file {0}
 countingObjects=Counting objects
+corruptPack=Pack file {0} is corrupt
 createBranchFailedUnknownReason=Create branch failed for unknown reason
 createBranchUnexpectedResult=Create branch returned unexpected result {0}
 createNewFileFailed=Could not create new file {0}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 5f42e81893..fd38dc1491 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -190,6 +190,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String corruptObjectNotree;
 	/***/ public String corruptObjectNoType;
 	/***/ public String corruptObjectPackfileChecksumIncorrect;
+	/***/ public String corruptPack;
 	/***/ public String couldNotCheckOutBecauseOfConflicts;
 	/***/ public String couldNotDeleteLockFileShouldNotHappen;
 	/***/ public String couldNotDeleteTemporaryIndexFileShouldNotHappen;
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 0b83efa62a..76fadefe53 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
@@ -66,6 +66,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackInvalidException;
 import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
@@ -330,9 +332,7 @@ public class ObjectDirectory extends FileObjectDatabase {
 				try {
 					p.resolve(matches, id, RESOLVE_ABBREV_LIMIT);
 				} catch (IOException e) {
-					// Assume the pack is corrupted.
-					logCorruptPackError(e, p);
-					removePack(p);
+					handlePackError(e, p);
 				}
 				if (matches.size() > RESOLVE_ABBREV_LIMIT)
 					return;
@@ -419,9 +419,7 @@ public class ObjectDirectory extends FileObjectDatabase {
 						if (searchPacksAgain(pList))
 							continue SEARCH;
 					} catch (IOException e) {
-						// Assume the pack is corrupted.
-						logCorruptPackError(e, p);
-						removePack(p);
+						handlePackError(e, p);
 					}
 				}
 				break SEARCH;
@@ -501,9 +499,7 @@ public class ObjectDirectory extends FileObjectDatabase {
 						if (searchPacksAgain(pList))
 							continue SEARCH;
 					} catch (IOException e) {
-						// Assume the pack is corrupted.
-						logCorruptPackError(e, p);
-						removePack(p);
+						handlePackError(e, p);
 					}
 				}
 				break SEARCH;
@@ -544,9 +540,7 @@ public class ObjectDirectory extends FileObjectDatabase {
 					pList = scanPacks(pList);
 					continue SEARCH;
 				} catch (IOException e) {
-					// Assume the pack is corrupted.
-					logCorruptPackError(e, p);
-					removePack(p);
+					handlePackError(e, p);
 				}
 			}
 			break SEARCH;
@@ -556,9 +550,19 @@ public class ObjectDirectory extends FileObjectDatabase {
 			h.db.selectObjectRepresentation(packer, otp, curs);
 	}
 
-	private static void logCorruptPackError(IOException e, PackFile p) {
-		StringBuilder buf = new StringBuilder(MessageFormat.format(
-				JGitText.get().exceptionWhileReadingPack,
+	private void handlePackError(IOException e, PackFile p) {
+		String tmpl;
+		if ((e instanceof CorruptObjectException)
+				|| (e instanceof PackInvalidException)) {
+			tmpl = JGitText.get().corruptPack;
+			// Assume the pack is corrupted, and remove it from the list.
+			removePack(p);
+		} else {
+			tmpl = JGitText.get().exceptionWhileReadingPack;
+			// Don't remove the pack from the list, as the error may be
+			// transient.
+		}
+		StringBuilder buf = new StringBuilder(MessageFormat.format(tmpl,
 				p.getPackFile().getAbsolutePath()));
 		StringWriter sw = new StringWriter();
 		e.printStackTrace(new PrintWriter(sw));