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 @@
*/
// 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 @@
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 @@
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 @@
/**
* 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 @@
} catch (IOException e) {
throw new JGitInternalException(e.getMessage(), e);
}
- if (autoGcBlockedByOldLockFile(background)) {
+ if (autoGcBlockedByOldLockFile()) {
lock.unlock();
return false;
}
