Wait opening new packfile until it can't be racy anymore

If
- pack.waitPreventRacyPack = true (default is false)
- packfile size > pack.minSizePreventRacyPack (default is 100 MB)
wait after a new packfile was written and before it is opened until it
cannot be racy anymore.

If a new packfile is accessed while it's still racy at least the pack's
index will be reread by ObjectDirectory.scanPacksImpl(). Hence it may
save resources to wait one tick of the file system timer to avoid this
reloading. On filesystems with a coarse timestamp resolution it may be
beneficial to skip this wait for small packfiles.

Bug: 546891
Change-Id: I0e8bf3d7677a025edd2e397dd2c9134ba59b1a18
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

6 files changed, 227 insertions, 6 deletions

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index a0fafdb1b0..524c591912 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -38,6 +38,62 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/storage/pack/PackConfig.java" type="org.eclipse.jgit.storage.pack.PackConfig">
+        <filter id="336658481">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.storage.pack.PackConfig"/>
+                <message_argument value="DEFAULT_MINSIZE_PREVENT_RACY_PACK"/>
+            </message_arguments>
+        </filter>
+        <filter id="336658481">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.storage.pack.PackConfig"/>
+                <message_argument value="DEFAULT_WAIT_PREVENT_RACY_PACK"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="DEFAULT_MINSIZE_PREVENT_RACY_PACK"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="DEFAULT_WAIT_PREVENT_RACY_PACK"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="doWaitPreventRacyPack(long)"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="getMinSizePreventRacyPack()"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="isWaitPreventRacyPack()"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="setMinSizePreventRacyPack(long)"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.1.8"/>
+                <message_argument value="setWaitPreventRacyPack(boolean)"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/transport/TransferConfig.java" type="org.eclipse.jgit.transport.TransferConfig">
         <filter id="1159725059">
             <message_arguments>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
index 23cd5ba310..b6837d2861 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
@@ -52,6 +52,7 @@ import java.time.Duration;
 import java.util.Date;
 import java.util.Locale;
 import java.util.Objects;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.util.FS;
@@ -287,6 +288,19 @@ public class FileSnapshot {
 	}
 
 	/**
+	 * Wait until this snapshot's file can't be racy anymore
+	 *
+	 * @throws InterruptedException
+	 *             if sleep was interrupted
+	 */
+	public void waitUntilNotRacy() throws InterruptedException {
+		while (isRacyClean(System.currentTimeMillis())) {
+			TimeUnit.NANOSECONDS
+					.sleep((fsTimestampResolution.toNanos() + 1) * 11 / 10);
+		}
+	}
+
+	/**
 	 * Compare two snapshots to see if they cache the same information.
 	 *
 	 * @param other
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 0c94043e18..3c830e88c1 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
@@ -1256,8 +1256,23 @@ public class GC {
 							realExt), e);
 				}
 			}
-
-			return repo.getObjectDatabase().openPack(realPack);
+			boolean interrupted = false;
+			try {
+				FileSnapshot snapshot = FileSnapshot.save(realPack);
+				if (pconfig.doWaitPreventRacyPack(snapshot.size())) {
+					snapshot.waitUntilNotRacy();
+				}
+			} catch (InterruptedException e) {
+				interrupted = true;
+			}
+			try {
+				return repo.getObjectDatabase().openPack(realPack);
+			} finally {
+				if (interrupted) {
+					// Re-set interrupted flag
+					Thread.currentThread().interrupt();
+				}
+			}
 		} finally {
 			if (tmpPack != null && tmpPack.exists())
 				tmpPack.delete();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryPackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryPackParser.java
index 0cec2d5a85..6e8a15e86d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryPackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryPackParser.java
@@ -65,6 +65,7 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.CoreConfig;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.storage.pack.PackConfig;
 import org.eclipse.jgit.transport.PackParser;
 import org.eclipse.jgit.transport.PackedObjectInfo;
 import org.eclipse.jgit.util.FileUtils;
@@ -122,9 +123,12 @@ public class ObjectDirectoryPackParser extends PackParser {
 	/** The pack that was created, if parsing was successful. */
 	private PackFile newPack;
 
+	private PackConfig pconfig;
+
 	ObjectDirectoryPackParser(FileObjectDatabase odb, InputStream src) {
 		super(odb, src);
 		this.db = odb;
+		this.pconfig = new PackConfig(odb.getConfig());
 		this.crc = new CRC32();
 		this.tailDigest = Constants.newMessageDigest();
 
@@ -514,6 +518,15 @@ public class ObjectDirectoryPackParser extends PackParser {
 					JGitText.get().cannotMoveIndexTo, finalIdx), e);
 		}
 
+		boolean interrupted = false;
+		try {
+			FileSnapshot snapshot = FileSnapshot.save(finalPack);
+			if (pconfig.doWaitPreventRacyPack(snapshot.size())) {
+				snapshot.waitUntilNotRacy();
+			}
+		} catch (InterruptedException e) {
+			interrupted = true;
+		}
 		try {
 			newPack = db.openPack(finalPack);
 		} catch (IOException err) {
@@ -523,6 +536,11 @@ public class ObjectDirectoryPackParser extends PackParser {
 			if (finalIdx.exists())
 				FileUtils.delete(finalIdx);
 			throw err;
+		} finally {
+			if (interrupted) {
+				// Re-set interrupted flag
+				Thread.currentThread().interrupt();
+			}
 		}
 
 		return lockMessage != null ? keep : null;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java
index 0ce3cc93ce..a27a2b00c3 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java
@@ -86,6 +86,7 @@ import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ObjectStream;
+import org.eclipse.jgit.storage.pack.PackConfig;
 import org.eclipse.jgit.transport.PackParser;
 import org.eclipse.jgit.transport.PackedObjectInfo;
 import org.eclipse.jgit.util.BlockList;
@@ -115,8 +116,11 @@ public class PackInserter extends ObjectInserter {
 	private PackStream packOut;
 	private Inflater cachedInflater;
 
+	private PackConfig pconfig;
+
 	PackInserter(ObjectDirectory db) {
 		this.db = db;
+		this.pconfig = new PackConfig(db.getConfig());
 	}
 
 	/**
@@ -296,9 +300,25 @@ public class PackInserter extends ObjectInserter {
 					realIdx), e);
 		}
 
-		db.openPack(realPack);
-		rollback = false;
-		clear();
+		boolean interrupted = false;
+		try {
+			FileSnapshot snapshot = FileSnapshot.save(realPack);
+			if (pconfig.doWaitPreventRacyPack(snapshot.size())) {
+				snapshot.waitUntilNotRacy();
+			}
+		} catch (InterruptedException e) {
+			interrupted = true;
+		}
+		try {
+			db.openPack(realPack);
+			rollback = false;
+		} finally {
+			clear();
+			if (interrupted) {
+				// Re-set interrupted flag
+				Thread.currentThread().interrupt();
+			}
+		}
 	}
 
 	private static void writePackIndex(File idx, byte[] packHash,
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
index 256e41d22b..6bd32dd873 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackConfig.java
@@ -116,12 +116,30 @@ public class PackConfig {
 	 */
 	public static final int DEFAULT_DELTA_SEARCH_WINDOW_SIZE = 10;
 
+	private static final int MB = 1 << 20;
+
 	/**
 	 * Default big file threshold: {@value}
 	 *
 	 * @see #setBigFileThreshold(int)
 	 */
-	public static final int DEFAULT_BIG_FILE_THRESHOLD = 50 * 1024 * 1024;
+	public static final int DEFAULT_BIG_FILE_THRESHOLD = 50 * MB;
+
+	/**
+	 * Default if we wait before opening a newly written pack to prevent its
+	 * lastModified timestamp could be racy
+	 *
+	 * @since 5.1.8
+	 */
+	public static final boolean DEFAULT_WAIT_PREVENT_RACY_PACK = false;
+
+	/**
+	 * Default if we wait before opening a newly written pack to prevent its
+	 * lastModified timestamp could be racy
+	 *
+	 * @since 5.1.8
+	 */
+	public static final long DEFAULT_MINSIZE_PREVENT_RACY_PACK = 100 * MB;
 
 	/**
 	 * Default delta cache size: {@value}
@@ -238,6 +256,10 @@ public class PackConfig {
 
 	private int bigFileThreshold = DEFAULT_BIG_FILE_THRESHOLD;
 
+	private boolean waitPreventRacyPack = DEFAULT_WAIT_PREVENT_RACY_PACK;
+
+	private long minSizePreventRacyPack = DEFAULT_MINSIZE_PREVENT_RACY_PACK;
+
 	private int threads;
 
 	private Executor executor;
@@ -314,6 +336,8 @@ public class PackConfig {
 		this.deltaCacheSize = cfg.deltaCacheSize;
 		this.deltaCacheLimit = cfg.deltaCacheLimit;
 		this.bigFileThreshold = cfg.bigFileThreshold;
+		this.waitPreventRacyPack = cfg.waitPreventRacyPack;
+		this.minSizePreventRacyPack = cfg.minSizePreventRacyPack;
 		this.threads = cfg.threads;
 		this.executor = cfg.executor;
 		this.indexVersion = cfg.indexVersion;
@@ -737,6 +761,76 @@ public class PackConfig {
 	}
 
 	/**
+	 * Get whether we wait before opening a newly written pack to prevent its
+	 * lastModified timestamp could be racy
+	 *
+	 * @return whether we wait before opening a newly written pack to prevent
+	 *         its lastModified timestamp could be racy
+	 * @since 5.1.8
+	 */
+	public boolean isWaitPreventRacyPack() {
+		return waitPreventRacyPack;
+	}
+
+	/**
+	 * Get whether we wait before opening a newly written pack to prevent its
+	 * lastModified timestamp could be racy. Returns {@code true} if
+	 * {@code waitToPreventRacyPack = true} and
+	 * {@code packSize > minSizePreventRacyPack}, {@code false} otherwise.
+	 *
+	 * @param packSize
+	 *            size of the pack file
+	 *
+	 * @return whether we wait before opening a newly written pack to prevent
+	 *         its lastModified timestamp could be racy
+	 * @since 5.1.8
+	 */
+	public boolean doWaitPreventRacyPack(long packSize) {
+		return isWaitPreventRacyPack()
+				&& packSize > getMinSizePreventRacyPack();
+	}
+
+	/**
+	 * Set whether we wait before opening a newly written pack to prevent its
+	 * lastModified timestamp could be racy
+	 *
+	 * @param waitPreventRacyPack
+	 *            whether we wait before opening a newly written pack to prevent
+	 *            its lastModified timestamp could be racy
+	 * @since 5.1.8
+	 */
+	public void setWaitPreventRacyPack(boolean waitPreventRacyPack) {
+		this.waitPreventRacyPack = waitPreventRacyPack;
+	}
+
+	/**
+	 * Get minimum packfile size for which we wait before opening a newly
+	 * written pack to prevent its lastModified timestamp could be racy if
+	 * {@code isWaitToPreventRacyPack} is {@code true}.
+	 *
+	 * @return minimum packfile size, default is 100 MiB
+	 *
+	 * @since 5.1.8
+	 */
+	public long getMinSizePreventRacyPack() {
+		return minSizePreventRacyPack;
+	}
+
+	/**
+	 * Set minimum packfile size for which we wait before opening a newly
+	 * written pack to prevent its lastModified timestamp could be racy if
+	 * {@code isWaitToPreventRacyPack} is {@code true}.
+	 *
+	 * @param minSizePreventRacyPack
+	 *            minimum packfile size, default is 100 MiB
+	 *
+	 * @since 5.1.8
+	 */
+	public void setMinSizePreventRacyPack(long minSizePreventRacyPack) {
+		this.minSizePreventRacyPack = minSizePreventRacyPack;
+	}
+
+	/**
 	 * Get the compression level applied to objects in the pack.
 	 *
 	 * Default setting: {@value java.util.zip.Deflater#DEFAULT_COMPRESSION}
@@ -1083,6 +1177,10 @@ public class PackConfig {
 		setBitmapInactiveBranchAgeInDays(
 				rc.getInt("pack", "bitmapinactivebranchageindays", //$NON-NLS-1$ //$NON-NLS-2$
 						getBitmapInactiveBranchAgeInDays()));
+		setWaitPreventRacyPack(rc.getBoolean("pack", "waitpreventracypack", //$NON-NLS-1$ //$NON-NLS-2$
+				isWaitPreventRacyPack()));
+		setMinSizePreventRacyPack(rc.getLong("pack", "minsizepreventracypack", //$NON-NLS-1$//$NON-NLS-2$
+				getMinSizePreventRacyPack()));
 	}
 
 	/** {@inheritDoc} */