Introduce core.trustLooseRefStat config

With repositories on NFS, JGit can read an old value of a loose ref
or miss the existence of a loose ref if file attributes of the loose
ref or its parent directories are cached by NFS. Introduce a new config
'core.trustLooseRefStat' that will optionally refresh file attributes of
the loose ref (at least on some NFS clients).

Possible values for this new config are:

* always: Trust loose ref file attributes (default)
* after_open: Similar to 'always', but refresh the file attributes of
              the loose ref and its parent directories before trusting
              it

The default is set to always trust the file attributes as after_open is
known to degrade performance.

In a subsequent change, SnapshottingRefDirectory will be updated to
cache the directories that were refreshed to avoid duplicate work and
thereby improve performance to some extent for the after_open setting.

Change-Id: I9dfaeaf5307b2b51ce6ee4bfd9e0678786685fcf
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>

diff --git a/Documentation/config-options.md b/Documentation/config-options.md
index ca94ab84d1194260869a5664e5b240a2f441fed3..78930e62676851f72b77ec09fcdb76a300cc2be4 100644 (file)
--- a/Documentation/config-options.md
+++ b/Documentation/config-options.md
@@ -56,6 +56,7 @@ For details on native git options see also the official [git config documentatio
 | `core.symlinks` | Auto detect if filesystem supports symlinks| ✅ | If false, symbolic links are checked out as small plain files that contain the link text. |
 | `core.trustFolderStat` | `true` | ⃞ | Whether to trust the pack folder's, packed-refs file's and loose-objects folder's file attributes (Java equivalent of stat command on *nix). When looking for pack files, if `false` JGit will always scan the `.git/objects/pack` folder and if set to `true` it assumes that pack files are unchanged if the file attributes of the pack folder are unchanged. When getting the list of packed refs, if `false` JGit will always read the packed-refs file and if set to `true` it uses the file attributes of the packed-refs file and will only read it if a file attribute has changed. When looking for loose objects, if `false` and if a loose object is not found, JGit will open and close a stream to `.git/objects` folder (which can refresh its directory listing, at least on some NFS clients) and retry looking for that loose object. Setting this option to `false` can help to workaround caching issues on NFS, but reduces performance. |
 | `core.trustPackedRefsStat` | `unset` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the packed-refs file. If `never` JGit will ignore the file attributes of the packed-refs file and always read it. If `always` JGit will trust the file attributes of the packed-refs file and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, except that the packed-refs file is opened and closed before its file attributes are considered. An open/close of the packed-refs file is known to refresh its file attributes, at least on some NFS clients. If `unset`, JGit will use the behavior described in `trustFolderStat`. |
+| `core.trustLooseRefStat` | `always` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the loose ref. If `always` JGit will trust the file attributes of the loose ref and its parent directories. `after_open` behaves similar to `always`, except that all parent directories of the loose ref up to the repository root are opened and closed before its file attributes are considered. An open/close of these parent directories is known to refresh the file attributes, at least on some NFS clients. |
 | `core.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | ✅ | The path to the root of the working tree. |
 
 ## __fetch__ options

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
index e27029798d1c136d4bce8dce5b7cd2c7f709dfed..169dce1cc0d926e20c491669c735d86c5923180b 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
@@ -37,6 +37,7 @@ import java.nio.file.DirectoryNotEmptyException;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.security.DigestInputStream;
 import java.security.MessageDigest;
 import java.text.MessageFormat;
@@ -64,6 +65,7 @@ import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.CoreConfig.TrustPackedRefsStat;
+import org.eclipse.jgit.lib.CoreConfig.TrustLooseRefStat;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.Ref;
@@ -181,6 +183,8 @@ public class RefDirectory extends RefDatabase {
 
        private final TrustPackedRefsStat trustPackedRefsStat;
 
+       private final TrustLooseRefStat trustLooseRefStat;
+
        RefDirectory(RefDirectory refDb) {
                parent = refDb.parent;
                gitDir = refDb.gitDir;
@@ -192,6 +196,7 @@ public class RefDirectory extends RefDatabase {
                packedRefs.set(refDb.packedRefs.get());
                trustFolderStat = refDb.trustFolderStat;
                trustPackedRefsStat = refDb.trustPackedRefsStat;
+               trustLooseRefStat = refDb.trustLooseRefStat;
                inProcessPackedRefsLock = refDb.inProcessPackedRefsLock;
        }
 
@@ -213,6 +218,10 @@ public class RefDirectory extends RefDatabase {
                                .getEnum(ConfigConstants.CONFIG_CORE_SECTION, null,
                                                ConfigConstants.CONFIG_KEY_TRUST_PACKED_REFS_STAT,
                                                TrustPackedRefsStat.UNSET);
+               trustLooseRefStat = db.getConfig()
+                               .getEnum(ConfigConstants.CONFIG_CORE_SECTION, null,
+                                               ConfigConstants.CONFIG_KEY_TRUST_LOOSE_REF_STAT,
+                                               TrustLooseRefStat.ALWAYS);
                inProcessPackedRefsLock = new ReentrantLock(true);
        }
 
@@ -1136,6 +1145,11 @@ public class RefDirectory extends RefDatabase {
 
        LooseRef scanRef(LooseRef ref, String name) throws IOException {
                final File path = fileFor(name);
+
+               if (trustLooseRefStat.equals(TrustLooseRefStat.AFTER_OPEN)) {
+                       refreshPathToLooseRef(Paths.get(name));
+               }
+
                FileSnapshot currentSnapshot = null;
 
                if (ref != null) {
@@ -1221,6 +1235,29 @@ public class RefDirectory extends RefDatabase {
                return new LooseUnpeeled(loose.snapshot, name, id);
        }
 
+       /**
+        * Workaround for issues caused by NFS caching. Refresh directories starting
+        * from the repository root to a loose ref by opening an input stream. This
+        * refreshes file attributes of the loose ref (at least on some NFS
+        * clients).
+        *
+        * @param refPath
+        *            path of a loose ref relative to the repository root
+        */
+       void refreshPathToLooseRef(Path refPath) {
+               for (int i = 1; i < refPath.getNameCount(); i++) {
+                       File dir = fileFor(refPath.subpath(0, i).toString());
+                       // Use Files.newInputStream(Path) as it is consistent with other
+                       // code where a refresh is being done (see getPackedRefs()) and also
+                       // it performs slightly better than Files.newDirectoryStream(Path)
+                       try (InputStream stream = Files.newInputStream(dir.toPath())) {
+                               // open the dir to refresh attributes (on some NFS clients)
+                       } catch (IOException e) {
+                               break; // loose ref may not exist
+                       }
+               }
+       }
+
        private static boolean isSymRef(byte[] buf, int n) {
                if (n < 6)
                        return false;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
index 76b4d7195e8b23abbbea120b9e60a63293733e22..0edf3c5ad026a9497f045715c7f5efa58fccc6f4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -971,6 +971,13 @@ public final class ConfigConstants {
         */
        public static final String CONFIG_KEY_TRUST_PACKED_REFS_STAT = "trustPackedRefsStat";
 
+       /**
+        * The "trustLooseRefStat" key
+        *
+        * @since 6.9
+        */
+       public static final String CONFIG_KEY_TRUST_LOOSE_REF_STAT = "trustLooseRefStat";
+
        /**
         * The "pack.preserveOldPacks" key
         *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
index 4de1801d04def691a278eec6bc10efc9d0d7530c..9fa5d75a3fe3b91c665cc9d35bcdc165b0ce522d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
@@ -144,6 +144,21 @@ public class CoreConfig {
                UNSET
        }
 
+       /**
+        * Permissible values for {@code core.trustLooseRefStat}.
+        *
+        * @since 6.9
+        */
+       public enum TrustLooseRefStat {
+
+               /** Trust file attributes of the loose ref. */
+               ALWAYS,
+
+               /** Open and close parent directories of the loose ref file until the
+                * repository root to refresh its file attributes and then trust it. */
+               AFTER_OPEN,
+       }
+
        private final int compression;
 
        private final int packIndexVersion;