Robustify the code for NFS weirdness #9
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Like the comment says, we want to support non-fsynced NFS-backed caches. They make a lot of sense when the NFS server is a reliable distributed service (e.g., FileStore on GCP or EFS on AWS), especially when the data also lives somewhere else (e.g., object storage). In such configurations, the chmod to mark files read-only before we make them visible in the cache directory also happens to flush any write buffered in the client: since filehandles are stateless inodes, the write requests could be rejected after the chmod (an interesting deviation from POSIX file semantics...)
pkhuong
force-pushed
the
pkhuong/nfs
branch
from
248c318
to
36e5fad
Compare
Most cases where we check for ENOENT, we want to bubble that up like a non-erroneous cache miss. ESTALE means we thought there was something there, but that handle (inode) has since disappeared, either because the file was unlinked or because it was replaced with another. Either way, it seems simplest to treat such stale filehandles like missing files. We don't expect this to happen a lot, except for cleanups: files in the kismet cache should mostly be write-once (content addressed), until the cache grows too large and old unused files are reaped to make room. In that situation, a stale filehandle means the file was deleted, and it's fine to treat that like ENOENT. Alternatively, one could also flush the client-side filehandle cache and retry, but that's more work for marginal benefits.
pkhuong
force-pushed
the
pkhuong/nfs
branch
from
36e5fad
to
79624ab
Compare
ceberly
approved these changes
Oct 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The write side actually worked fine (not deliberately). We also expect to see stale file handles when reaping unused files from overfull caches. Treat that like ENOENT.