From baf1bb20d02f60d001a4d77d9749f845d4ada45e Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Mon, 11 Feb 2019 07:49:43 +0000 Subject: [PATCH 1/7] ObjectDirectory: extra logging on packfile exceptions Display extra logging, including the exception with the associated stacktrace, whenever a packFile can't be read and thus removed from the packlist. Change-Id: I97a4e31dc427bfcc0baae438dcbe2dcd4704b824 Signed-off-by: Luca Milanesio (cherry picked from commit 962babc4b27ffd90058fe7734f17ed1c4e77d958) --- .../org/eclipse/jgit/internal/storage/file/ObjectDirectory.java | 2 ++ 1 file changed, 2 insertions(+) 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 153c7dd92..6b8ef3a1b 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 @@ -355,6 +355,7 @@ public class ObjectDirectory extends FileObjectDatabase { // The hasObject call should have only touched the index, // so any failure here indicates the index is unreadable // by this process, and the pack is likewise not readable. + LOG.warn("Unable to read packfile " + p.getPackFile(), e); removePack(p); } } @@ -647,6 +648,7 @@ public class ObjectDirectory extends FileObjectDatabase { if ((e instanceof CorruptObjectException) || (e instanceof PackInvalidException)) { warnTmpl = JGitText.get().corruptPack; + LOG.warn("Packfile " + p.getPackFile() + " is corrupted", e); // Assume the pack is corrupted, and remove it from the list. removePack(p); } else if (e instanceof FileNotFoundException) { From 90abad1baaef544312b27df73adb80c9fada26e3 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sun, 3 Mar 2019 13:14:46 +0900 Subject: [PATCH 2/7] Bazel: Stop using native.git_repository The native.git_repository method doesn't work in the latest version of bazel, and causes the build to fail with: type 'struct' has no method git_repository() Change-Id: Id6a57369b681c0afe811e9e3740b141fb7fb4653 Signed-off-by: David Pursehouse (cherry picked from commit ec5fc57b791081fa073fc5fd91286347238f8f7c) --- tools/bazlets.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/bazlets.bzl b/tools/bazlets.bzl index f97b72c82..f089af473 100644 --- a/tools/bazlets.bzl +++ b/tools/bazlets.bzl @@ -1,10 +1,12 @@ +load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") + NAME = "com_googlesource_gerrit_bazlets" def load_bazlets( commit, local_path = None): if not local_path: - native.git_repository( + git_repository( name = NAME, remote = "https://gerrit.googlesource.com/bazlets", commit = commit, From e6fd4732d0836f54e97dd87e7a1c15862d789376 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 25 Feb 2019 08:35:12 +0900 Subject: [PATCH 3/7] ObjectDirectory: Clean up logging Externalize the message and log the pack file with absolute path. Change-Id: I019052dfae8fd96ab67da08b3287d699287004cb Signed-off-by: David Pursehouse (cherry picked from commit 9665d86ba1dd2937ca26f6aba63bb16aa277f888) --- .../org/eclipse/jgit/internal/JGitText.properties | 1 + .../src/org/eclipse/jgit/internal/JGitText.java | 1 + .../jgit/internal/storage/file/ObjectDirectory.java | 7 +++++-- 3 files changed, 7 insertions(+), 2 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 51a0ca971..d29151b37 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -689,6 +689,7 @@ truncatedHunkOldLinesMissing=Truncated hunk, at least {0} old lines is missing tSizeMustBeGreaterOrEqual1=tSize must be >= 1 unableToCheckConnectivity=Unable to check connectivity. unableToCreateNewObject=Unable to create new object: {0} +unableToReadPackfile=Unable to read packfile {0} unableToRemovePath=Unable to remove path ''{0}'' unableToStore=Unable to store {0}. unableToWrite=Unable to write {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 67fca5e85..2fc5501f4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -748,6 +748,7 @@ public class JGitText extends TranslationBundle { /***/ public String tSizeMustBeGreaterOrEqual1; /***/ public String unableToCheckConnectivity; /***/ public String unableToCreateNewObject; + /***/ public String unableToReadPackfile; /***/ public String unableToRemovePath; /***/ public String unableToStore; /***/ public String unableToWrite; 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 6b8ef3a1b..e953bbe30 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 @@ -355,7 +355,9 @@ public class ObjectDirectory extends FileObjectDatabase { // The hasObject call should have only touched the index, // so any failure here indicates the index is unreadable // by this process, and the pack is likewise not readable. - LOG.warn("Unable to read packfile " + p.getPackFile(), e); + LOG.warn(MessageFormat.format( + JGitText.get().unableToReadPackfile, + p.getPackFile().getAbsolutePath()), e); removePack(p); } } @@ -648,7 +650,8 @@ public class ObjectDirectory extends FileObjectDatabase { if ((e instanceof CorruptObjectException) || (e instanceof PackInvalidException)) { warnTmpl = JGitText.get().corruptPack; - LOG.warn("Packfile " + p.getPackFile() + " is corrupted", e); + LOG.warn(MessageFormat.format(warnTmpl, + p.getPackFile().getAbsolutePath()), e); // Assume the pack is corrupted, and remove it from the list. removePack(p); } else if (e instanceof FileNotFoundException) { From daefa69502ec58cc8a28151364b80b56b918f2de Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sat, 23 Feb 2019 21:57:09 +0000 Subject: [PATCH 4/7] PackFile: report correct message for checksum mismatch When the packfile checksum does not match the expected one report the correct checksum error instead of reporting that the number of objects is incorrect. Change-Id: I040f36dacc4152ae05453e7acbf8dfccceb46e0d Signed-off-by: Luca Milanesio Signed-off-by: Matthias Sohn (cherry picked from commit 436c99ce5946f31f06b8704b1fd33136f39dc814) --- .../org/eclipse/jgit/internal/JGitText.properties | 2 +- .../eclipse/jgit/internal/storage/file/PackFile.java | 11 ++++++----- 2 files changed, 7 insertions(+), 6 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 d29151b37..9fab96c18 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -489,7 +489,7 @@ openingConnection=Opening connection operationCanceled=Operation {0} was canceled outputHasAlreadyBeenStarted=Output has already been started. overflowedReftableBlock=Overflowed reftable block -packChecksumMismatch=Pack checksum mismatch detected for pack file {0} +packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: .pack has {1} whilst .idx has {2} packCorruptedWhileWritingToFilesystem=Pack corrupted while writing to filesystem packDoesNotMatchIndex=Pack {0} does not match index packedRefsHandleIsStale=packed-refs handle is stale, {0}. retry 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 0611d3e85..58c314130 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 @@ -187,7 +187,8 @@ public class PackFile implements Iterable { } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { throw new PackMismatchException(MessageFormat.format( JGitText.get().packChecksumMismatch, - packFile.getPath())); + packFile.getPath(), packChecksum, + idx.packChecksum)); } loadedIdx = idx; } catch (InterruptedIOException e) { @@ -750,10 +751,10 @@ public class PackFile implements Iterable { fd.readFully(buf, 0, 20); if (!Arrays.equals(buf, packChecksum)) { throw new PackMismatchException(MessageFormat.format( - JGitText.get().packObjectCountMismatch - , ObjectId.fromRaw(buf).name() - , ObjectId.fromRaw(idx.packChecksum).name() - , getPackFile())); + JGitText.get().packChecksumMismatch, + getPackFile(), + ObjectId.fromRaw(buf).name(), + ObjectId.fromRaw(idx.packChecksum).name())); } } From e8bd9556c0c43e272a8d1d105f1108a2568e2482 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 7 Jul 2017 00:58:28 +0200 Subject: [PATCH 5/7] Cancel gc if thread was interrupted see https://groups.google.com/d/msg/repo-discuss/oDB2rl3doDc/tFEh5Xt0CAAJ Change-Id: Ia6d4631c64e065d8b9b09e0b45e7a9ea8ac3f41d Signed-off-by: Matthias Sohn (cherry picked from commit 882fed0d96c533513c43ae77aaff9cc07b94012c) --- .../storage/file/GcConcurrentTest.java | 52 +++++++++++++++++++ .../jgit/internal/storage/file/GC.java | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java index 643bb4946..c60c357da 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java @@ -47,27 +47,35 @@ import static java.lang.Integer.valueOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.errors.CancelledException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.pack.PackWriter; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.EmptyProgressMonitor; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Sets; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.junit.Test; public class GcConcurrentTest extends GcTestCase { @@ -221,4 +229,48 @@ public class GcConcurrentTest extends GcTestCase { assertEquals(getSinglePack(repository).getPackName(), newPackName); assertNotNull(getSinglePack(repository).getBitmapIndex()); } + + @Test + public void testInterruptGc() throws Exception { + FileBasedConfig c = repo.getConfig(); + c.setInt(ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTOPACKLIMIT, 1); + c.save(); + SampleDataRepositoryTestCase.copyCGitTestPacks(repo); + ExecutorService executor = Executors.newSingleThreadExecutor(); + final CountDownLatch latch = new CountDownLatch(1); + Future> result = executor + .submit(new Callable>() { + + @Override + public Collection call() throws Exception { + long start = System.currentTimeMillis(); + System.out.println("starting gc"); + latch.countDown(); + Collection r = gc.gc(); + System.out.println("gc took " + + (System.currentTimeMillis() - start) + " ms"); + return r; + } + }); + try { + latch.await(); + Thread.sleep(5); + executor.shutdownNow(); + result.get(); + fail("thread wasn't interrupted"); + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + if (cause instanceof CancelledException) { + assertEquals(JGitText.get().operationCanceled, + cause.getMessage()); + } else if (cause instanceof IOException) { + Throwable cause2 = cause.getCause(); + assertTrue(cause2 instanceof InterruptedException + || cause2 instanceof ExecutionException); + } else { + fail("unexpected exception " + e); + } + } + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 9300f022b..1b62c14dd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1280,7 +1280,7 @@ public class GC { } private void checkCancelled() throws CancelledException { - if (pm.isCancelled()) { + if (pm.isCancelled() || Thread.currentThread().isInterrupted()) { throw new CancelledException(JGitText.get().operationCanceled); } } From e8f7bb3608589edc06d80605e2157ff68a9719b6 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 4 Mar 2019 13:28:25 +0100 Subject: [PATCH 6/7] Properly format pack checksums in PackFile.idx() Change-Id: Id805850dbe9a3d633168f3056e06ddeafd86f961 Signed-off-by: Matthias Sohn (cherry picked from commit a33e4dc58a87daf34072b82643aba0fd4456c165) --- .../src/org/eclipse/jgit/internal/storage/file/PackFile.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 58c314130..d28c04fd6 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 @@ -187,8 +187,9 @@ public class PackFile implements Iterable { } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { throw new PackMismatchException(MessageFormat.format( JGitText.get().packChecksumMismatch, - packFile.getPath(), packChecksum, - idx.packChecksum)); + packFile.getPath(), + ObjectId.fromRaw(packChecksum).name(), + ObjectId.fromRaw(idx.packChecksum).name())); } loadedIdx = idx; } catch (InterruptedIOException e) { From 7b3ee6f62e0cd0d3437abb12f9488dedd8af5125 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 4 Mar 2019 13:44:24 +0100 Subject: [PATCH 7/7] Fix error log message in ObjectDirectory.handlePackError() Change-Id: I154f392ad025c4b642eb1123d375a0afaa853885 Signed-off-by: Matthias Sohn (cherry picked from commit 997d785418d55dce5a1188fdb95e6d2b4ab0bde5) --- .../resources/org/eclipse/jgit/internal/JGitText.properties | 2 +- .../eclipse/jgit/internal/storage/file/ObjectDirectory.java | 4 ++-- 2 files changed, 3 insertions(+), 3 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 9fab96c18..b02430b09 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -279,7 +279,7 @@ exceptionCaughtDuringExecutionOfTagCommand=Exception caught during execution of 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, {1}. Caught {2} consecutive errors while trying to read this pack. +exceptionWhileReadingPack=Exception caught while accessing pack file {0}, the pack file might be corrupt. Caught {1} 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 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 e953bbe30..200b60df5 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 @@ -681,8 +681,8 @@ public class ObjectDirectory extends FileObjectDatabase { // 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); + p.getPackFile().getAbsolutePath(), + Integer.valueOf(transientErrorCount)), e); } } }