Refresh 'objects' dir and retry if a loose object is not found

A new loose object may not be immediately visible on a NFS
client if it was created on another client. Refreshing the
'objects' dir and trying again can help work around the NFS
behavior.

Here's an E2E problem that this change can help fix. Consider
a Gerrit multi-primary setup with repositories based on NFS.
Add a new patch-set to an existing change and then immediately
fetch the new patch-set of that change. If the fetch is handled
by a Gerrit primary different that the one which created the
patch-set, then we sometimes run into a MissingObjectException
that causes the fetch to fail.

Bug: 581317
Change-Id: Iccc6676c68ef13a1e8b2ff52b3eeca790a89a13d
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
This commit is contained in:
Kaushik Lingarkar 2023-01-11 21:25:30 -08:00 committed by Matthias Sohn
parent 82b5aaf7e3
commit fed1a54935
3 changed files with 66 additions and 9 deletions

View File

@ -45,7 +45,7 @@ For details on native git options see also the official [git config documentatio
| `core.streamFileThreshold` | `50 MiB` | &#x20DE; | The size threshold beyond which objects must be streamed. |
| `core.supportsAtomicFileCreation` | `true` | &#x20DE; | Whether the filesystem supports atomic file creation. |
| `core.symlinks` | Auto detect if filesystem supports symlinks| &#x2705; | If false, symbolic links are checked out as small plain files that contain the link text. |
| `core.trustFolderStat` | `true` | &#x20DE; | Whether to trust the pack folder's and packed-refs file'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. Setting this option to `false` can help to workaround caching issues on NFS, but reduces performance.|
| `core.trustFolderStat` | `true` | &#x20DE; | 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` | &#x20DE; | 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.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | &#x2705; | The path to the root of the working tree. |

View File

@ -14,6 +14,7 @@
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.StandardCopyOption;
@ -24,6 +25,8 @@
import org.eclipse.jgit.internal.storage.file.FileObjectDatabase.InsertLooseObjectResult;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
@ -52,15 +55,22 @@ class LooseObjects {
private final UnpackedObjectCache unpackedObjectCache;
private final boolean trustFolderStat;
/**
* Initialize a reference to an on-disk object directory.
*
* @param config
* configuration for the loose objects handler.
* @param dir
* the location of the <code>objects</code> directory.
*/
LooseObjects(File dir) {
LooseObjects(Config config, File dir) {
directory = dir;
unpackedObjectCache = new UnpackedObjectCache();
trustFolderStat = config.getBoolean(
ConfigConstants.CONFIG_CORE_SECTION,
ConfigConstants.CONFIG_KEY_TRUSTFOLDERSTAT, true);
}
/**
@ -98,6 +108,19 @@ boolean hasCached(AnyObjectId id) {
* @return {@code true} if the specified object is stored as a loose object.
*/
boolean has(AnyObjectId objectId) {
boolean exists = hasWithoutRefresh(objectId);
if (trustFolderStat || exists) {
return exists;
}
try (InputStream stream = Files.newInputStream(directory.toPath())) {
// refresh directory to work around NFS caching issue
} catch (IOException e) {
return false;
}
return hasWithoutRefresh(objectId);
}
private boolean hasWithoutRefresh(AnyObjectId objectId) {
return fileFor(objectId).exists();
}
@ -183,6 +206,22 @@ ObjectLoader open(WindowCursor curs, AnyObjectId id) throws IOException {
*/
ObjectLoader getObjectLoader(WindowCursor curs, File path, AnyObjectId id)
throws IOException {
try {
return getObjectLoaderWithoutRefresh(curs, path, id);
} catch (FileNotFoundException e) {
if (trustFolderStat) {
throw e;
}
try (InputStream stream = Files
.newInputStream(directory.toPath())) {
// refresh directory to work around NFS caching issues
}
return getObjectLoaderWithoutRefresh(curs, path, id);
}
}
private ObjectLoader getObjectLoaderWithoutRefresh(WindowCursor curs,
File path, AnyObjectId id) throws IOException {
try (FileInputStream in = new FileInputStream(path)) {
unpackedObjectCache().add(id);
return UnpackedObject.open(in, path, id, curs);
@ -203,16 +242,34 @@ UnpackedObjectCache unpackedObjectCache() {
}
long getSize(WindowCursor curs, AnyObjectId id) throws IOException {
try {
return getSizeWithoutRefresh(curs, id);
} catch (FileNotFoundException noFile) {
try {
if (trustFolderStat) {
throw noFile;
}
try (InputStream stream = Files
.newInputStream(directory.toPath())) {
// refresh directory to work around NFS caching issue
}
return getSizeWithoutRefresh(curs, id);
} catch (FileNotFoundException e) {
if (fileFor(id).exists()) {
throw noFile;
}
unpackedObjectCache().remove(id);
return -1;
}
}
}
private long getSizeWithoutRefresh(WindowCursor curs, AnyObjectId id)
throws IOException {
File f = fileFor(id);
try (FileInputStream in = new FileInputStream(f)) {
unpackedObjectCache().add(id);
return UnpackedObject.getSize(in, id, curs);
} catch (FileNotFoundException noFile) {
if (f.exists()) {
throw noFile;
}
unpackedObjectCache().remove(id);
return -1;
}
}

View File

@ -120,7 +120,7 @@ public ObjectDirectory(final Config cfg, final File dir,
File packDirectory = new File(objects, "pack"); //$NON-NLS-1$
File preservedDirectory = new File(packDirectory, "preserved"); //$NON-NLS-1$
alternatesFile = new File(objects, Constants.INFO_ALTERNATES);
loose = new LooseObjects(objects);
loose = new LooseObjects(config, objects);
packed = new PackDirectory(config, packDirectory);
preserved = new PackDirectory(config, preservedDirectory);
this.fs = fs;