From 0bf25644339ecd150645e85becc9012d724ba6fb Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 29 Jan 2020 18:55:43 +0100 Subject: [PATCH 1/5] reftable: clarify that LogCursor may return a null ReflogEntry Change-Id: I1a4d5c262cd196dca37876aec00bb974a45e9fcd Signed-off-by: Han-Wen Nienhuys --- .../org/eclipse/jgit/internal/storage/reftable/LogCursor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java index 486fd2898..5c98242f1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/LogCursor.java @@ -45,6 +45,7 @@ package org.eclipse.jgit.internal.storage.reftable; import java.io.IOException; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.lib.ReflogEntry; /** @@ -78,8 +79,9 @@ public abstract class LogCursor implements AutoCloseable { /** * Get current log entry. * - * @return current log entry. + * @return current log entry. Maybe null if we are producing deletions. */ + @Nullable public abstract ReflogEntry getReflogEntry(); /** {@inheritDoc} */ From dd203f03c2ac8444b6c4804653c829a986660973 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 29 Jan 2020 18:56:12 +0100 Subject: [PATCH 2/5] reftable: remove outdated comment Signed-off-by: Han-Wen Nienhuys Change-Id: I41a7d9934d53553b5f81f40ff9d663378676ac5d --- .../eclipse/jgit/internal/storage/file/FileReftableStack.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java index 2c416c380..dc55ccc40 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java @@ -285,9 +285,6 @@ public class FileReftableStack implements AutoCloseable { } if (!success) { - // TODO: should reexamine the 'refs' file to see if it was the same - // if it didn't change, then we must have corruption. If it did, - // retry. throw new LockFailedException(stackPath); } From 8c9f7656c3078a4d4a2986400a9700a9c13b416f Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 29 Jan 2020 19:11:00 +0100 Subject: [PATCH 3/5] reftable: clear cache on full compaction The merged table contains handles to open files. A full compaction causes those files to be closed, and so further lookups would fail with EBADF. Change-Id: I7bb74f7228ecc7fec9535b00e56a617a9c18e00e Signed-off-by: Han-Wen Nienhuys Signed-off-by: Matthias Sohn --- .../storage/file/FileReftableTest.java | 38 +++++++++++++++++++ .../storage/file/FileReftableDatabase.java | 1 + 2 files changed, 39 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java index cdc64fa1b..bca113fbf 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java @@ -59,6 +59,7 @@ import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; +import java.security.SecureRandom; import java.util.ArrayList; import java.util.List; @@ -542,6 +543,43 @@ public class FileReftableTest extends SampleDataRepositoryTestCase { assertEquals(RefUpdate.Result.LOCK_FAILURE, rename.rename()); } + @Test + public void compactFully() throws Exception { + FileReftableDatabase refDb = (FileReftableDatabase) db.getRefDatabase(); + PersonIdent person = new PersonIdent("jane", "jane@invalid"); + + ObjectId aId = db.exactRef("refs/heads/a").getObjectId(); + ObjectId bId = db.exactRef("refs/heads/b").getObjectId(); + + SecureRandom random = new SecureRandom(); + List strs = new ArrayList<>(); + for (int i = 0; i < 1024; i++) { + strs.add(String.format("%02x", + Integer.valueOf(random.nextInt(256)))); + } + + String randomStr = String.join("", strs); + String refName = "branch"; + for (long i = 0; i < 2; i++) { + RefUpdate ru = refDb.newUpdate(refName, false); + ru.setNewObjectId(i % 2 == 0 ? aId : bId); + ru.setForceUpdate(true); + // Only write a large string in the first table, so it becomes much larger + // than the second, and the result is not autocompacted. + ru.setRefLogMessage(i == 0 ? randomStr : "short", false); + ru.setRefLogIdent(person); + + RefUpdate.Result res = ru.update(); + assertTrue(res == Result.NEW || res == FORCED); + } + + assertEquals(refDb.exactRef(refName).getObjectId(), bId); + assertTrue(randomStr.equals(refDb.getReflogReader(refName).getReverseEntry(1).getComment())); + refDb.compactFully(); + assertEquals(refDb.exactRef(refName).getObjectId(), bId); + assertTrue(randomStr.equals(refDb.getReflogReader(refName).getReverseEntry(1).getComment())); + } + @Test public void reftableRefsStorageClass() throws IOException { Ref b = db.exactRef("refs/heads/b"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java index fd80ad9ec..c9ee165a3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java @@ -139,6 +139,7 @@ public class FileReftableDatabase extends RefDatabase { reftableDatabase.getLock().lock(); try { reftableStack.compactFully(); + reftableDatabase.clearCache(); } finally { reftableDatabase.getLock().unlock(); } From 936a031ca341c4ffb16939350f81adadf5c2d7dd Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 29 Jan 2020 19:11:22 +0100 Subject: [PATCH 4/5] reftable: clarify comment Change-Id: I16e32aeb325b604eb31f84db18a214f792395941 Signed-off-by: Han-Wen Nienhuys Signed-off-by: Matthias Sohn --- .../eclipse/jgit/internal/storage/file/FileReftableStack.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java index dc55ccc40..4536e0463 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java @@ -396,7 +396,7 @@ public class FileReftableStack implements AutoCloseable { * * @param w * writer to write data to a reftable under construction - * @return true if the transaction. + * @return true if the transaction was successful. * @throws IOException * on I/O problems */ From 79266a1fe5b6893ee66177489f7f59f08cb2b294 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Wed, 29 Jan 2020 19:12:06 +0100 Subject: [PATCH 5/5] reftable: don't check deadline on the first try This helps debug stepping. Change-Id: I020dafab4ffac75e6df0dbcde0ed4805c7867f72 Signed-off-by: Han-Wen Nienhuys --- .../jgit/internal/storage/file/FileReftableStack.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java index 4536e0463..cc52bfb71 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java @@ -262,8 +262,13 @@ public class FileReftableStack implements AutoCloseable { long max = 1000; long delay = 0; boolean success = false; - while (System.currentTimeMillis() < deadline) { + + // Don't check deadline for the first 3 retries, so we can step with a + // debugger without worrying about deadlines. + int tries = 0; + while (tries < 3 || System.currentTimeMillis() < deadline) { List names = readTableNames(); + tries++; try { reloadOnce(names); success = true;