-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FUSE Passthrough Support in Stargz-Snapshotter #1867 #1868
Conversation
Signed-off-by: abushwang <[email protected]>
Signed-off-by: abushwang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Could you fix linter errors? https://github.com/containerd/stargz-snapshotter/actions/runs/11906582478/job/33178914643?pr=1868
fs/config/config.go
Outdated
@@ -148,4 +148,7 @@ type FuseConfig struct { | |||
|
|||
// EntryTimeout defines TTL for directory, name lookup in seconds. | |||
EntryTimeout int64 `toml:"entry_timeout"` | |||
|
|||
// PassThrough indicates whether to enable FUSE passthrough mode to improve local file read performance. Default is false. | |||
PassThrough bool `toml:"passthrough"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we test this in our CI?
b.Reset() | ||
b.Grow(int(chunkSize)) | ||
ip := b.Bytes()[:chunkSize] | ||
if _, err := sf.fr.ReadAt(ip, chunkOffset); err != nil && err != io.EOF { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we check if the data can be read from the cache?
@@ -82,6 +82,7 @@ type BlobCache interface { | |||
type Reader interface { | |||
io.ReaderAt | |||
Close() error | |||
GetReaderAt() io.ReaderAt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment like If a blob is backed by a file, it should return *os.File so that it can be used for FUSE passthrough.
?
fs/layer/node.go
Outdated
n.fs.s.report(fmt.Errorf("node.Open: %v", err)) | ||
return nil, 0, syscall.EIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we continue opening this as a non-passthrough file, instead of returning EIO?
@@ -82,6 +82,7 @@ type BlobCache interface { | |||
type Reader interface { | |||
io.ReaderAt | |||
Close() error | |||
GetReaderAt() io.ReaderAt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When FUSE passthrough is enabled, we should always set directoryCache.direct
to true
so that we can ensure that *directoryCache.Get
always return *os.File (not a buffer).
3fd3113
to
5dba826
Compare
all done @ktock |
This is my benchmark I create three sets of files with sizes of 50, 75, and 100MB respectively. Subsequently, I used the default value for the --estargz-chunk-size parameter to create an estargz image.
To ensure a clean environment for each test, I removed the local image and cache, and then restarted the containerd-stargz-grpc service.
This is the report:
env:
|
cmd/containerd-stargz-grpc/main.go
Outdated
// isFusePthEnable prevents users from enabling passthrough mode on unsupported kernel versions | ||
func isFusePthEnable() (bool, error) { | ||
cmd := exec.Command("sh", "-c", "grep 'CONFIG_FUSE_PASSTHROUGH=y' /boot/config-$(uname -r)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Let's try not to rely on the shell commands for now. Instead of having this check, let's put a document about how to check if passthrough is a supported on the node (maybe in the following PRs)
Signed-off-by: abushwang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Does the kernel version need to be updated to 6.9 to test out passthrough? |
yes,or rebase this feature |
Here’s a proposed implementation plan for FUSE passthrough:
By following this approach, subsequent Read operations would not need to return to user space, and go-fuse would release the registered information when necessary.
for details,
#1867