kappa-db / multifeed

Multi-writer hypercore.
135 stars 27 forks source link

Feature/Bugfix: Live Feed Announcements #28

Closed telamon closed 5 years ago

telamon commented 5 years ago

See issue #27

hackergrrl commented 5 years ago

There is not! Heck, not all streams even emit 'end' :D

The safest bet is to use a module like end-of-stream to reliably detect it ending.

On 05/13 02:01, Tony Ivanov wrote:

telamon commented on this pull request.

@@ -213,6 +215,16 @@ Multifeed.prototype.replicate = function (opts) { var keys = values(self._feeds).map(function (feed) { return feed.key.toString('hex') }) mux.haveFeeds(keys) }) +

  • if(opts.live) {
  • // Push session to _streams array
  • self._streams.push(mux)
  • // Register removal
  • mux.stream.on('end', function(err) {

Hmm.. must be my memory failing, but wasn't there a single event that got emitted regardless of success/error result? If not then I'll add on('error') as well.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/kappa-db/multifeed/pull/28#discussion_r283248729

telamon commented 5 years ago

@noffle oh damn. that's sad. Got some bad news from my end, spent the evening yesterday tweaking the issue and I'm afraid that I have redesign the execution flow of events in order to support more than one feed-announcement + exchange.

Currently multifeed works like this:

A -- connect --> B
A -- send manifest --> B
A <-- want subset -- B    // set flag remoteReady && try replicate()
A <-- send manifest -- B
A -- want subset --> B // set flag localReady && try replicate()
when both ends are 'ready' replicateFeeds().

Not entirely unexpected, the flags (presence of _localWantand _remoteWants) cause a lot of unexpected behavior in a scenarios where each end might send more have and 'want` requests asynchronically, especially if one end receives 2 consecutive manifests. ugh

So current idea is to redesign the protocol into individual asynchroneous processes:

local -- send 'offer' --> remote
local <-- receive 'request' -- remote
local.replicateFeeds(request - offer)
local -- send ACK 'request - offer' --> remote

This will sadly require a protocol version bump.. but we'll get rid of the corruptible state.

telamon commented 5 years ago

Progress update I've successfully refactored the feed exchange protocol into a simplified and async-friendly version that implements the same replication sentiment.

So now multiple feed-exchanges can safely run in parallel on the same socket and can be initiated at any point in time during the socket's lifetime.

Multifeed protocol 3.0.0 Feed Announcement & Exchange

mermaid-diagram-multifeed-exchange-3 0 0

All integration tests pass, but I still need to update the previous 'mux' unit tests which as expected fail due to protocol redesign.

telamon commented 5 years ago

@noffle I feel pretty satisfied with this PR now. Please review when you have time :)

hackergrrl commented 5 years ago

I will! Thanks so much for this :D

On 05/16 14:55, Tony Ivanov wrote:

@noffle I feel pretty satisfied with this PR now. Please review when you have time :)

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/kappa-db/multifeed/pull/28#issuecomment-493245463

hackergrrl commented 5 years ago

That illustration is :kissing_heart::ok_hand: We should put it in a doc!

hackergrrl commented 5 years ago

Eeeeee I think it's ready!

You are a :star: @telamon. Fantastic PR.

hackergrrl commented 5 years ago

4.0.0 (major due to protocol bump)