hanwen / go-fuse

FUSE bindings for Go
Other
2.05k stars 328 forks source link

Go-FUSE logs are ambiguous with multiple servers #484

Closed Kern-- closed 1 year ago

Kern-- commented 1 year ago

We use go-fuse in soci-snapshotter - a containerd snapshotter that enables lazy loading of unmodified container images. Containerd creates a separate mount for each layer in a container image so that containers that share layers can share the content. For our implementation, this means each container layer has it's own fuse.Server which are layered using OverlayFS.

I'd like to be able to use the go-fuse logs to help debug workloads where lazy loading is performing poorly. Specifically, I'd like to be able to use the go-fuse logs to reconstruct all of the filesystem operations and their timings (we're adding timing information to the default logger) to either fix our implementation or drive optimizations in the workload itself.

Unfortunately, the unique id in the logs is only unique to a specific server so we end up seeing duplicates.

I propose adding an optional serverID string to fuse.Server. When logging operations, if the ID is not the empty, then add it in parenthases (e.g.):

rx 1(1): ... 
tx 1(1): ...
rx 1(2): ...
tx 1(2): ...

I'm happy to send a PR if you think this is a decent idea.

navytux commented 1 year ago

@Kern--, thanks for bringing this up.

To me your case suggests that instead of adding support for particular use case of serverID, we better add ability to generally specify log.Logger for fuse.Server, so that it is both possible to a) control where logs are sinked, and b) to add a custom prefix. The messages would then look like e.g.

srv1: rx 1: ...
srv1: tx 1: ...
srv2: rx 1: ...
srv2: tx 2: ...

@hanwen, @rfjakob, do you think that makes sense?

Kirill

Kern-- commented 1 year ago

Yeah, that seems like a better solution and it would definitely solve my use-case.

hanwen commented 1 year ago

fixed in https://github.com/hanwen/go-fuse/commit/7b4f97c3577b3b799babe8a6cf3eebe41dde7193

navytux commented 1 year ago

Thanks, @hanwen.