Datastore interface is a bottleneck

[crackcomm]

Hey, I just want to raise an issue that datastore interface is a bottleneck for high-performance usage eq. in FUSE mount because it doesn't provide Seek or partial read function with offset and size.

[welcome]

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

• "Priority" labels will show how urgent this is for the team.
• "Status" labels will show if this is ready to be worked on, blocked, or in progress.
• "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

[crackcomm]

See rendered cpuprof from FUSE file access.

The file is read 100x for a single read.

This in turn creates a huge strain on the garbage collector.

Performance could also be potentially improved by using buffer pools.

[crackcomm]

It might be beneficial to introduce the concept of the Open or seekable method that could be used in FUSE.

The seekable method can be provided for any byte array, see Rust peekable for an example implementation.

The method Get returning []byte for DAGs is fine but we can do better than allocating 250x1MB of []byte to read a 1MB file while reading in 4KB chunks.

In Go, see also SectionReader.

[aschmahmann]

This doesn't really seem like a datastore problem as much as an issue in the FUSE implementation.

Sure you could create a SeekableDatastore interface that extends Datastore and then implement that in some relevant datastores.

However, the application layer (i.e. your FUSE implementation) probably has a better idea of what it's trying to do than a datastore might. For example, if a block of data is 1MiB and you're reading it in 4KiB chunks, but ultimately are likely to be reading the full block of data, you could just read the 1MiB from disk and hold it until the file descriptor closes or some LRU cache is exceeded.

Given that most of the current implementations of the datastore interface that I know of do not implement Seek, or Open those would have to be implemented.

If you could implement this more efficiently in the application layer we could then figure out if and how it might be extracted to work across more applications.

Note: If you're working with the go-ipfs FUSE implementation I agree that it could use a number of improvements (performance and otherwise). You may want to check through this issue to see some work that was done. There was a contributor working on improving FUSE, but it he hasn't had the time to push things along. Perhaps you'd be interested in helping out.

[djdv]

@aschmahmann

There was a contributor working on improving FUSE, but it he hasn't had the time to push things along.

That's not the case :^/
I asked the former maintainer to meet with me to discuss design decisions until they stopped responding. Around August I was trying to schedule with you over Slack for a few weeks but nothing ever came of it.

I'm available to work on this if it's something the project wants, but I need someone with authority to discuss direction and decisions with me as well as just generally review things from time to time.
I have a link to the notes about this here: ipfs/kubo#7575 (comment)
An example of the type of things that need to be discussed are things such as what should the config format look like and things of similar scope.
I'm willing to do the work on things, I just need to know what direction to go in.

Feel free to reach out and we can collaborate on this.

[aschmahmann]

Oh hey @djdv. Didn't mean to offend you at all, sorry if came off like that. If I left you hanging I apologize.

I've been a bit swamped, but things are starting to clear up and we're making plans for next year. I'll reach out so we can catch up and figure out the next steps.

[crackcomm]

This in fact might be seen as a problem of go-ipfs FUSE implementation but this will be inherent to any application using datastore interface. This is the reason for the issue.

I also think that datastore user applications could be improved as @aschmahmann said but this will be putting lipstick on a pig.

In the case of ds-flatfs, current implementation can be easily improved in many ways.

@djdv, because of the garbage collector in Go, optimizations are less obvious than in most other languages.

If the user can't manage the []byte, the interface problem will continue creating more and more strain on GC.

[crackcomm]

I think some might be concerned about backward compatibility and the issue of porting all libraries at once, this is not necessary when changing it in such a small way. A new datastore method can be added to the interface, keeping Get in place and adding wrappers with NewReader, NopCloser etc.

The main problem is allocating new byte slices on each call to Get. Perfectly we would return buffer.Bytes() from Get using a wrapper in the end, not vice-versa.

In turn, this would allow us to provide alternative lazy-loading of values returned by the datastore interface.

The same happens when you open a file on your file system.

[aschmahmann]

FlatFS is only one of many datastores using these interfaces (Badger, LevelDB, S3, Azure, Memory, Pebble, ...) there's nothing wrong with adding more to go-ds-flatfs and then checking for optimizations if the datastore supports it.

If it turns out more than one or two databases support the given functionality we can talk about adding a new extension interface (e.g. TTLStore), but until then Go let's you just check the interface type without needing to declare that interface globally.

[crackcomm]

@aschmahmann indeed maybe that could be extension using Go interface casting.

[djdv]

@crackcomm

See rendered cpuprof from FUSE file access.
The file is read 100x for a single read.

Curious to see what the flamegraph looks like on this branch: https://github.com/djdv/go-ipfs/tree/tmp
Would you be willing to run it?

It might be beneficial to introduce the concept of the Open or seekable method that could be used in FUSE.

I don't remember much about the original fuse implementation's use of the Datastore API directly, but I can say that in the branch above, a FileSystem interface is defined which is used to interface with FUSE and other host APIs.
The "Files" returned from it have seeking as part of their interface.

I suppose these things are considered to be a bridge between the 'File System' and 'Application' layers.

For example, we're implementing fuse, as an application ipfs mount. Our implementation utilizes the APIs of the existing filesystems, such as Unix FS, MFS, and other CoreAPI methods which all incorporate their own forms of seeking, caching, flushing, etc.
Either through their API, or added/wrapped in some way by us.
Rather than manipulating the datastore directly ourselves (in our application layer).

A good example of this is the UFS wrapper
There is some complexity around making a reference counted, shared object, but ultimately we're just wrapping the existing dag modifier library, which implements its own cache, seeking, flushing, etc. and we just pass requests through to it.

[aschmahmann]

As was mentioned above given that this request is coming from an application layer request (a FUSE implementation of go-ipfs wants to be able to read partial file data) and that there are a large number of steps and refactors that would be required in order to provide that functionality natively rather than on top of the application layer (e.g. by just caching the bytes instead of repeatedly retrieving them) I'm going to close this issue.

For context some of the refactors that might be required to make this work include:

• Refactor all in use datastores to allow partial retrieval
  • If it turns out some of the existing datastores don't support it then the application layer needs to figure out how to deal with both gracefully -> 😢
• Refactor all consumer interfaces and structs that consume the datastore to expose partial retrieval (e.g. blockstore and implementations)
  • Make sure not to run into issues with the blockstore no longer outputting verifiable data that when hashed matches a CID
• Refactor application layer to take advantage of partial retrieval

If someone is interested in this I'd recommend going from the application layer downwards rather than from the lowest layer interface upwards. Go makes it so you can just make new interfaces and do type checking on them rather than needing to change the base interface declaration which makes this even easier to do.