From fed5b2e6fb12a5ade484a83c20c31d57f5630559 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 19 Jul 2020 12:00:35 +0200 Subject: Remove unused imports Change-Id: I7c44e3603df2dd368cb7c0ba0072413b887b6903 Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java | 1 - org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java | 1 - 2 files changed, 2 deletions(-) (limited to 'org.eclipse.jgit.test/tst') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java index bfa30d5b4b..b5c73c2b02 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource; -import org.eclipse.jgit.internal.storage.dfs.DfsRefDatabase; import org.eclipse.jgit.internal.storage.reftable.RefCursor; import org.eclipse.jgit.internal.storage.reftable.ReftableConfig; import org.eclipse.jgit.internal.storage.reftable.ReftableReader; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java index 8b253828c4..0303bb4f7c 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java @@ -45,7 +45,6 @@ package org.eclipse.jgit.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import org.eclipse.jgit.util.Stats; import org.junit.Test; public class StatsTest { -- cgit v1.2.3 From fde7a271a4f961484ae6e8357fd64277cfc03585 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 25 Nov 2020 17:00:09 +0100 Subject: Ensure that GC#deleteOrphans respects pack lock If pack or index files are guarded by a pack lock (.keep file) deleteOrphans() should not touch the respective files protected by the lock file. Otherwise it may interfere with PackInserter concurrently inserting a new pack file and its index. The problem was caused by the following race. All mentioned files are located in "objects/pack/". File endings relevant in "pack" dir: .pack .keep .idx .bitmap When ReceivePack receives a pack file it executes the following steps: ReceivePack.service(): receivePackAndCheckConnectivity(): receivePack(): receive the pack parse the pack, returns packLock (.keep file) PackInserter.flush(): write tmpPck file: "insert_.pack" write tmpIdx file: "insert_.idx" real pack name: "pack-.pack" real index name: "pack-.idx" atomic rename tmpPack to realPack atomic rename tmpIdx to tmpIdx execute commands unlock pack by removing .keep file trigger auto gc if enabled When PackInserter.flush() renames the temporary pack to the final "pack-xxx.pack" file the temporary pack index file "insert_xxx.idx" has no matching .pack file with the same base name for a short interval. If deleteOrphans() ran during that interval it deduced the pack index file was orphaned. Subsequently the missing pack index caused MissingObjectExceptions since objects contained in the pack couldn't be looked up anymore. Bug: https://bugs.chromium.org/p/gerrit/issues/detail?id=13544 Change-Id: I559c81e4b1d7c487f92a751bd78b987d32c98719 Signed-off-by: Matthias Sohn --- .../internal/storage/file/GcOrphanFilesTest.java | 24 ++++++++++++++++++++-- .../org/eclipse/jgit/internal/storage/file/GC.java | 9 ++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) (limited to 'org.eclipse.jgit.test/tst') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcOrphanFilesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcOrphanFilesTest.java index 79d72c56d0..e54c4ad458 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcOrphanFilesTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcOrphanFilesTest.java @@ -57,11 +57,15 @@ public class GcOrphanFilesTest extends GcTestCase { private final static String BITMAP_File_1 = PACK + "-1.bitmap"; - private final static String IDX_File_2 = PACK + "-2.idx"; + private static final String BITMAP_File_2 = PACK + "-2.bitmap"; + + private static final String IDX_File_2 = PACK + "-2.idx"; private final static String IDX_File_malformed = PACK + "-1234idx"; - private final static String PACK_File_2 = PACK + "-2.pack"; + private static final String KEEP_File_2 = PACK + "-2.keep"; + + private static final String PACK_File_2 = PACK + "-2.pack"; private final static String PACK_File_3 = PACK + "-3.pack"; @@ -105,6 +109,22 @@ public class GcOrphanFilesTest extends GcTestCase { assertTrue(new File(packDir, IDX_File_malformed).exists()); } + @Test + public void keepPreventsDeletionOfIndexFilesForMissingPackFile() + throws Exception { + createFileInPackFolder(BITMAP_File_1); + createFileInPackFolder(IDX_File_2); + createFileInPackFolder(BITMAP_File_2); + createFileInPackFolder(KEEP_File_2); + createFileInPackFolder(PACK_File_3); + gc.gc(); + assertFalse(new File(packDir, BITMAP_File_1).exists()); + assertTrue(new File(packDir, BITMAP_File_2).exists()); + assertTrue(new File(packDir, IDX_File_2).exists()); + assertTrue(new File(packDir, KEEP_File_2).exists()); + assertTrue(new File(packDir, PACK_File_3).exists()); + } + private void createFileInPackFolder(String fileName) throws IOException { if (!packDir.exists() || !packDir.isDirectory()) { assertTrue(packDir.mkdirs()); 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 791a108289..25c5dcf8a5 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 @@ -148,6 +148,8 @@ public class GC { private static final String INDEX_EXT = "." + PackExt.INDEX.getExtension(); //$NON-NLS-1$ + private static final String KEEP_EXT = "." + PackExt.KEEP.getExtension(); //$NON-NLS-1$ + private static final int DEFAULT_AUTOPACKLIMIT = 50; private static final int DEFAULT_AUTOLIMIT = 6700; @@ -978,7 +980,10 @@ public class GC { fileNames = files.map(path -> path.getFileName().toString()) .filter(name -> (name.endsWith(PACK_EXT) || name.endsWith(BITMAP_EXT) - || name.endsWith(INDEX_EXT))) + || name.endsWith(INDEX_EXT) + || name.endsWith(KEEP_EXT))) + // sort files with same base name in the order: + // .pack, .keep, .index, .bitmap to avoid look ahead .sorted(Collections.reverseOrder()) .collect(Collectors.toList()); } catch (IOException e1) { @@ -990,7 +995,7 @@ public class GC { String base = null; for (String n : fileNames) { - if (n.endsWith(PACK_EXT)) { + if (n.endsWith(PACK_EXT) || n.endsWith(KEEP_EXT)) { base = n.substring(0, n.lastIndexOf('.')); } else { if (base == null || !n.startsWith(base)) { -- cgit v1.2.3