search

kappa-db / multifeed

Multi-writer hypercore.
135 stars 27 forks source link

feat: Add `removeFeed(name|key)` #56

Open lejeunerenard opened 3 years ago

lejeunerenard commented 3 years ago

Here is an initial stab at implementing removeFeed(). It takes a name or a key and will remove the feed from the multifeed and destroy it (which clears the storage as well as closes the feed).

This implements #19 but without the blacklist mechanism. I felt that this should be a separate mechanism/feature.

Potential Issue

Currently there is one know issue with the way storage works. A storage is created per feed and is named sequentially which is problematic for persistent storage modules. When loading feeds in _loadFeeds(), it is assumed the feeds are sequential and that everything has been loaded when it hits a storage that does not load. So if we remove a storage and then reload feeds, it will ignore all feeds after the removed feed.

A potential solution is to add another storage instance to store an index of all feeds and defaulting to sequential names for legacy support when the index storage isn't given / loaded.

lejeunerenard commented 3 years ago

I added an implementation of the storage index. It feels a bit clunky (especially the legacy support in loading the feeds), so I am open to any suggestions. However, from my testing, it all works.

lejeunerenard commented 3 years ago

To summarize my storage solution, I create a comma separated list of current storage directories (one per feed) in a storage index directory with a name dirs. This file is updated every time a feed is added via creating a new write() or via replication. The index file is then used to load feeds in _loadFeeds() if it exists. If it doesn't exist, then the default sequential numerical loading method (ie. load 0, 1, etc until it doesnt load) is used instead.

The fact that it falls back to the original sequential format when no index is found, but automatically create an index if the feeds are modified enables legacy support that gracefully upgrades.

hackergrrl commented 3 years ago

@lejeunerenard This is an exciting set of changes to have in multifeed! Thank you for taking the time to put this together in a PR. 🎈

I've been pretty overloaded lately re: reviewing. Is this a change that you're depending on / using in your own projects, or non-blocking for you?

lejeunerenard commented 3 years ago

@hackergrrl I am not depending on it. There is still an issue in my storage solution that I've also been too busy to find out the bug. I do hope to get to it soon however. Feel free to ignore this PR for now since you are busy and it's not ready.