Fix GC run in foreground to not use executor Since I3870cadb4, GC task was always delegated to an executor even when background option was set to false. This was an issue because if more than one GC object was instantiated and executed in parallel, only one GC was actually running because of the single thread executor. Change-Id: I8c587d22d63c1601b7d75914692644a385cd86d6 Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
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 d99a485..e4b0a46 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
@@ -78,16 +78,13 @@ import java.util.Set; import java.util.TreeMap; import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.Future; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jgit.annotations.NonNull; -import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.errors.CancelledException; import org.eclipse.jgit.errors.CorruptObjectException; @@ -232,8 +229,11 @@ public GC(FileRepository repo) { */ // TODO(ms): in 5.0 change signature and return Future<Collection<PackFile>> public Collection<PackFile> gc() throws IOException, ParseException { - final GcLog gcLog = background ? new GcLog(repo) : null; - if (gcLog != null && !gcLog.lock(background)) { + if (!background) { + return doGc(); + } + final GcLog gcLog = new GcLog(repo); + if (!gcLog.lock()) { // there is already a background gc running return Collections.emptyList(); } @@ -241,48 +241,31 @@ public Collection<PackFile> gc() throws IOException, ParseException { Callable<Collection<PackFile>> gcTask = () -> { try { Collection<PackFile> newPacks = doGc(); - if (automatic && tooManyLooseObjects() && gcLog != null) { + if (automatic && tooManyLooseObjects()) { String message = JGitText.get().gcTooManyUnpruned; gcLog.write(message); gcLog.commit(); } return newPacks; } catch (IOException | ParseException e) { - if (background) { - if (gcLog == null) { - // Lacking a log, there's no way to report this. - return Collections.emptyList(); - } - try { - gcLog.write(e.getMessage()); - StringWriter sw = new StringWriter(); - e.printStackTrace(new PrintWriter(sw)); - gcLog.write(sw.toString()); - gcLog.commit(); - } catch (IOException e2) { - e2.addSuppressed(e); - LOG.error(e2.getMessage(), e2); - } - } else { - throw new JGitInternalException(e.getMessage(), e); + try { + gcLog.write(e.getMessage()); + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + gcLog.write(sw.toString()); + gcLog.commit(); + } catch (IOException e2) { + e2.addSuppressed(e); + LOG.error(e2.getMessage(), e2); } } finally { - if (gcLog != null) { - gcLog.unlock(); - } + gcLog.unlock(); } return Collections.emptyList(); }; - Future<Collection<PackFile>> result = executor.submit(gcTask); - if (background) { - // TODO(ms): in 5.0 change signature and return the Future - return Collections.emptyList(); - } - try { - return result.get(); - } catch (InterruptedException | ExecutionException e) { - throw new IOException(e); - } + // TODO(ms): in 5.0 change signature and return the Future + executor.submit(gcTask); + return Collections.emptyList(); } private Collection<PackFile> doGc() throws IOException, ParseException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java index 9ea77cc..35049d4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java
@@ -44,20 +44,17 @@ package org.eclipse.jgit.internal.storage.file; import org.eclipse.jgit.api.errors.JGitInternalException; -import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.util.GitDateParser; import org.eclipse.jgit.util.SystemReader; import static java.nio.charset.StandardCharsets.UTF_8; -import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.FileTime; -import java.text.MessageFormat; import java.text.ParseException; import java.time.Instant; @@ -103,22 +100,11 @@ private Instant getLogExpiry() throws ParseException { return gcLogExpire; } - private boolean autoGcBlockedByOldLockFile(boolean background) { + private boolean autoGcBlockedByOldLockFile() { try { FileTime lastModified = Files.getLastModifiedTime(logFile.toPath()); if (lastModified.toInstant().compareTo(getLogExpiry()) > 0) { // There is an existing log file, which is too recent to ignore - if (!background) { - try (BufferedReader reader = Files - .newBufferedReader(logFile.toPath())) { - char[] buf = new char[1000]; - int len = reader.read(buf, 0, 1000); - String oldError = new String(buf, 0, len); - - throw new JGitInternalException(MessageFormat.format( - JGitText.get().gcLogExists, oldError, logFile)); - } - } return true; } } catch (NoSuchFileException e) { @@ -132,11 +118,9 @@ private boolean autoGcBlockedByOldLockFile(boolean background) { /** * Lock the GC log file for updates * - * @param background - * If true, and if gc.log already exists, unlock and return false * @return {@code true} if we hold the lock */ - boolean lock(boolean background) { + boolean lock() { try { if (!lock.lock()) { return false; @@ -144,7 +128,7 @@ boolean lock(boolean background) { } catch (IOException e) { throw new JGitInternalException(e.getMessage(), e); } - if (autoGcBlockedByOldLockFile(background)) { + if (autoGcBlockedByOldLockFile()) { lock.unlock(); return false; }