mcollina / hyperemitter

Horizontally Scalable EventEmitter powered by a Merkle DAG
ISC License
70 stars 2 forks source link

Calling emit more than once in sync fashion causes idemopotency #2

Closed mcdonnelldean closed 9 years ago

mcdonnelldean commented 9 years ago

@mcollina documenting this here.

Given the following message:

message user {
 required int32 id = 1; 
}

If we send this over the emitter like so:

var user =  {
 id: 1 
}

emitter.emit('user', user)
emitter.emit('user', user)

The second message will be lost. This is to do with how the ._last value works. If we do:

var user =  {
 id: 1 
}

emitter.emit('user', user, function () {
  emitter.emit('user', user);
})

Both messages will be received.

mcollina commented 9 years ago

Yes, hypermitter is built on hyperlog, in a fashion that all publishes are concatenated with the others. This simplifies replication.

The true problem is that writes are serialized in hyperlog, but we were disccussing about batching https://github.com/mafintosh/hyperlog/issues/2.

Pulling @mafintosh here, as he might have some insights :).

mafintosh commented 9 years ago

@mcollina you could just change https://github.com/mcollina/hyperemitter/blob/master/hyperemitter.js#L130 to this

this._hyperlog.heads(function (err, heads) {
  if (err) return cb(err)
  self._hyperlog.add(heads, header, cb)
})

to fix it.

mcollina commented 9 years ago

That does not solve it, because the two heads call will run in parallel, and will return the exact same thing :(.

At this point I'm inclined in adding a random number to the event. The other option is adding a append method to hyperlog, which behave exactly like said, but within the lock (mutexify is not reentrant, so I cannot call lock from outside).

mafintosh commented 9 years ago

@mcollina hyperlog 3.5.0 has an append method that does this now.

mcollina commented 9 years ago

That was fast!!!!! Thanks @mafintosh!

mafintosh commented 9 years ago

Your use-case was valid :+1: . Fun fact, you should be able to replicate hyperemitters using dat (beta) since they share the replication layer :) Maybe we even use this for some sort of job management in dat.

mcdonnelldean commented 9 years ago

Awesome I've changed docs and eg samples too since this goes away

mcollina commented 9 years ago

If you need it, we might split the connection logic from the event processing logic, so that dat can use its own. The connection logic could be done better anyway, and these are two different responsibilities. Il giorno lun 4 mag 2015 alle 19:59 Dean McDonnell notifications@github.com ha scritto:

Awesome I've changed docs and eg samples too since this goes away

— Reply to this email directly or view it on GitHub https://github.com/mcollina/hyperemitter/issues/2#issuecomment-98797450.

mcdonnelldean commented 9 years ago

@mcollina you mean split into different repos or just split it out code wise?

mcdonnelldean commented 9 years ago

Also what is dat?

mcollina commented 9 years ago

repos! Basically one that builds only on hyperlog, without listen/connect and the EventPeer id, so that it can be used in dat as a 'control plane' thing (given they have their own connection management). The other module provides the listen/connect but it separate from HyperEmitter. Il giorno lun 4 mag 2015 alle 20:19 Dean McDonnell notifications@github.com ha scritto:

@mcollina https://github.com/mcollina you mean split into different repos or just split it out code wise?

— Reply to this email directly or view it on GitHub https://github.com/mcollina/hyperemitter/issues/2#issuecomment-98803074.

mcdonnelldean commented 9 years ago

Do you mean this repo?

https://github.com/maxogden/dat

Is there much point having two repos over one that is broken down with two ways of running? The repo with the connection bits wont be useful on it's own?