From 21f7fdff79ce2863aba40bc2b676059884a3a8e9 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Thu, 22 Feb 2024 15:29:48 -0800 Subject: [PATCH] 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 --- Documentation/config-options.md | 1 + .../internal/storage/file/RefDirectory.java | 37 +++++++++++++++++++ .../org/eclipse/jgit/lib/ConfigConstants.java | 7 ++++ .../src/org/eclipse/jgit/lib/CoreConfig.java | 15 ++++++++ 4 files changed, 60 insertions(+) diff --git a/Documentation/config-options.md b/Documentation/config-options.md index ca94ab84d1..78930e6267 100644 --- 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 e27029798d..169dce1cc0 100644 --- 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 76b4d7195e..0edf3c5ad0 100644 --- 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 4de1801d04..9fa5d75a3f 100644 --- 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; -- 2.39.5