About FakeBus sample implementation and contracts for the registration
beachwalker opened this issue · 13 comments
Even though I know this should be seen as a demonstration for learning CQRS and is just the most simplest implementation... I have seen people jump onto this horse and start to ride. Because of this I wan't to take a note on the implementation of Command, Event, FakeBus and the registration of a handler.
Both Command and Event share the same base class Message here. I don't think you should go this path, especially when looking at the registration.
There are two different contracts for Command registration and Event registration.
- You can publish to multiple (Event)handlers,
- but you are not allowed to send a Command to multiple (Command)handlers, there can be only one.
You will notice this here in the FakeBus implementation:
public void Send<T>(T command) where T : Command
{
List<Action<Message>> handlers;
if (_routes.TryGetValue(typeof(T), out handlers))
{
if (handlers.Count != 1) // look at this, only 1 handler allowed
throw new InvalidOperationException("cannot send to more than one handler");
handlers[0](command);
}
else
{
throw new InvalidOperationException("no handler registered");
}
}... on the other hand the Publish method does not have such a restriction.
But using the RegisterHandler<T>(Action<T> handler) where T : Message method you were already able to register as much as Command handlers for the same Command as you want:
public void RegisterHandler<T>(Action<T> handler) where T : Message
{
List<Action<Message>> handlers;
if(!_routes.TryGetValue(typeof(T), out handlers))
{
handlers = new List<Action<Message>>();
_routes.Add(typeof(T), handlers);
}
// no rules applied to check and fail early when trying to register 2nd Command handler
handlers.Add((x => handler((T)x)));
}Because of this, the RegisterHandler<T>(Action<T> handler) where T : Message should be really something like RegisterEventHandler<T>(Action<T> handler) where T : Event and RegisterCommandHandler<T>(Action<T> handler) where T : Command.
This way you are able to express and check the required rules earlier in the registration process (e.g. throw Exception during startup instead of running a long time with a hidden bug).
I would not even make Command and Event inherit from a shared base class at this point (any reasons to give?). The implemented FakeBus itself might be capable of transport both and might be the same instance. Greg already created two different interfaces, ICommandSender and IEventPublisher so you are able to keep them in separate implementations that wrap a common transport bus.
send a PR. This code was written over the course of 2 days as a quick sample :)
I know, just because I have seen this is popular to start with for experiments with the CQRS pattern... and "prototypes" can have a long lifetime. ;-)
Congrats for being able to write this in two days... I don't think I'm able to do that in such a short time, because even though CQRS is "just" a pattern/method it is much more complex than a Visitor or Singleton and involves multiple pitfalls.
I have found some bugs when this system is put under stress (many commands/events fired). I have done some changes and will send a PR.
Anyway, thank you very much for sharing this nice demo code!!!
I wouldn't recommend using this setup in production
On Wed, Dec 2, 2015 at 5:24 PM, Thomas Stegemann notifications@github.com
wrote:
I know, just because I have seen this is popular to start with for
experiments with the CQRS pattern... and "prototypes" can have a long
lifetime. ;-)Congrats for being able to write this in two days... I don't think I'm
able to do that in such a short time, because even though CQRS is "just" a
pattern/method it is much more complex than a Visitor or Singleton and
involves multiple pitfalls.I have found some bugs when this system is put under stress (many
commands/events fired). I have done some changes and will send a PR.Anyway, thank you very much for sharing this nice demo code!!!
—
Reply to this email directly or view it on GitHub
#13 (comment).
Studying for the Turing test
I know it is far away from production code, but it is a good thing to learn the principle of CQRS.
A real world project might miss Snapshots, Sagas, Threading, etc. but it was an interesting journey to see were the "optimization" potential in CQRS might be, especially when put under stress with generated data instead of slow user input. Your sample is small enough for changing things quickly (no legacy burden), but complete enough to work for testing purposes and to see what might become a problem.
For example I replaced the Command/Event Bus (maybe because of this I wrote the issue here, too) and used Reactive Extensions instead. The code was also my "testbed" for threading implementation tests. It is quite more complicated to start trial of an idea on a legacy brown field project.
Sorry, I turned this into a chat. ;-)
@gregoryyoung I'm a big fan of your work and totally recognize that this is just an example. But, I think even as example code goes, this will lead people down the garden path. By that I mean an authoritative figure such as yourself merely displaying an example that commits to an event store, then conceptually publishes to a bus for view model construction is going to teach people how to implement ES (in the larger sense with out-of-process denormalizers etc) incorrectly. This isn't due to the example purely being a naive example. I think the example is actually a misrepresentation of the pattern once you go out-of-process with an ESB (which is suggestive in your example given you've named your bus "FakeBus"). Many people will cleanup your example - substituting the FakeBus for a real ESB - and won't even realize they have a huge problem in there code now (due to the 2PC issue that ensues). I think part of the problem may be by naming it "FakeBus", people are given the impression that "oh I'll just replace this with my real bus". Perhaps if it was more obvious that this pattern is only safe if they use this example in an isolated runtime, there'd be a lot of people realizing that firstly: an ESB is not required here and secondly: subscribers to the produced events should be tailing off the event store (maybe Kafka or something treating the event store as a commit log that can be subscribed to, and replayed from). Bottom line, I think there should be a big fat disclaimer that says, IF YOU REPLACE THE FAKEBUS WITH A REAL BUS, THROW OUT THIS EXAMPLE AS IT NO LONGER APPLIES! I think despite your best intentions, unfortunately people are going to take this example and do exactly what I described above, and it needs to be made obvious that not only is this code not production code, the example is very very naive, and the FakeBus cannot simple be replaced with a real one.
Anyways, keep up the good work.
Indeed there are several implementations of frameworks based upon this sample, e.g. CQRSlite. So if there are Bugs in the code this might lead people to think that CQRS+ES itself has conceptual Problems at first sight (which is not the case). @asiraky Can you explain which part you mean with 2 phase commit?
The issue is there is effectively a distributed transaction occurring here. One operation happens on the event store (saving the event), and then there's a second operation (publishing of the event over a potential bus). If the first succeeds and the second fails, then you end up recording the event but not publishing the event. This can lead to varying issues depending on the types of things that subscribers are doing, and likewise there are lots of solutions to this problem, but none are simple when implemented properly. Solving the problem with a 2PC can be quite complex and increase latency, as the nodes need to agree over a distributed network when it's ok to commit the transaction. This is basically what Microsoft's DTC service does under the covers for you, hence why using MSMQ as a transport often makes solving the problem easy. Ideally you should just be avoiding this architecture.
And putting that aside, if you do ES properly, you don't need a bus or broker that publishes events to queues, then throws them away. Greg Young has spoken about this before, and essentially the point is your subscribers should be dictating the subscriptions to your event stream, allowing you to add new components coming into the ecosystem by simply subscribing to a position in the stream. Kafka is one option, when configured to retain events. This means you simply publish to Kafka which is the event stream, and it stores the events and forwards them to subscribers. The key being the subscriber controls the subscription by maintaining where they are up to in the stream.
I think @asiraky refers to the part where you have to guarantee that the events you are saving are also successfully published to the bus, or fail altogether. But still, the worst thing about ESB could be what I saw @gregoryyoung explain in a conference: what happen when you have a new subscriber that needs all the historical events and not just the new ones? You end up with a huge pile of accidental complexity. And you will need, more often than not, new read models that depends on all your events, including the ones that are currently happening when you process the older ones. When you get rid of the ESB, then you can treat your Event Store as a queue and with a consumer driven subscription approach you can poll (maybe with the long polling technique) for new events, starting from the first one and then play catch-up embracing eventual consistency.
Im sorry for my poor english grammar, I am from South America, and my mother tongue is spanish
@Narvalex - yes, this is exactly what I'm talking about. You shouldn't need an ESB and an ES. Just an ES that can be subscribed to for historical events. Then any distributed transactions go away.
Submit a patch (its a relatively trivial one) just make the projections
read directly from the event store as opposed to the publish operation
On Thu, Dec 17, 2015 at 11:23 AM, asiraky notifications@github.com wrote:
The issue is there is effectively a distributed transaction occurring
here. One operation happens on the event store (saving the event), and then
there's a second operation (publishing of the event over a potential bus).
If the first succeeds and the second fails, then you end up recording the
event but not publishing the event. This can lead to varying issues
depending on the types of things that subscribers are doing, and likewise
there are lots of solutions to this problem, but none are simple when
implemented properly. Solving the problem with a 2PC can be quite complex
and increase latency, as the nodes need to agree over a distributed network
when it's ok to commit the transaction. This is basically what Microsoft's
DTC service does under the covers for you, hence why using MSMQ as a
transport often makes solving the problem easy. Ideally you should just be
avoiding this architecture.And putting that aside, if you do ES properly, you don't need a bus or
broker that publishes events to queues, then throws them away. Greg Young
has spoken about this before, and essentially the point is your subscribers
should be dictating the subscriptions to your event stream, allowing you to
add new components coming into the ecosystem by simply subscribing to a
position in the stream. Kafka is one option, when configured to retain
events. This means you simply publish to Kafka which is the event stream,
and it stores the events and forwards them to subscribers. The key being
the subscriber controls the subscription by maintaining where they are up
to in the stream.—
Reply to this email directly or view it on GitHub
#13 (comment).
Studying for the Turing test
Hi, I think you mistakenly tagged my email in this but I'm not the asiraki
or asiraky you're thinking of so please remove me. Thanks.
On 17 December 2015 at 08:52, Greg Young notifications@github.com wrote:
Submit a patch (its a relatively trivial one) just make the projections
read directly from the event store as opposed to the publish operationOn Thu, Dec 17, 2015 at 11:23 AM, asiraky notifications@github.com
wrote:The issue is there is effectively a distributed transaction occurring
here. One operation happens on the event store (saving the event), and
then
there's a second operation (publishing of the event over a potential
bus).
If the first succeeds and the second fails, then you end up recording the
event but not publishing the event. This can lead to varying issues
depending on the types of things that subscribers are doing, and likewise
there are lots of solutions to this problem, but none are simple when
implemented properly. Solving the problem with a 2PC can be quite complex
and increase latency, as the nodes need to agree over a distributed
network
when it's ok to commit the transaction. This is basically what
Microsoft's
DTC service does under the covers for you, hence why using MSMQ as a
transport often makes solving the problem easy. Ideally you should just
be
avoiding this architecture.And putting that aside, if you do ES properly, you don't need a bus or
broker that publishes events to queues, then throws them away. Greg Young
has spoken about this before, and essentially the point is your
subscribers
should be dictating the subscriptions to your event stream, allowing you
to
add new components coming into the ecosystem by simply subscribing to a
position in the stream. Kafka is one option, when configured to retain
events. This means you simply publish to Kafka which is the event
stream,
and it stores the events and forwards them to subscribers. The key being
the subscriber controls the subscription by maintaining where they are up
to in the stream.—
Reply to this email directly or view it on GitHub
#13 (comment).Studying for the Turing test
—
Reply to this email directly or view it on GitHub
#13 (comment).
Actually, I never thought to do that. I will do that and submit it, as you're right it is quite simple. It would just require making the in memory store you have observable.
What about the race conditions between Creational and Updating/Changing events. They have an event handler but what to do if creation event is not finished processing before the update? Do I miss something? It seems to be a part of the CQRS that the Handlers for events should be processed in order of apearance of the events, too. So you need something like an EventProcessor (EventBus) that does not inform the subscribers of subsequent events (e.g. ItemNameChanged) before the former (ItemCreated) is completely processed (like await finish of operation). Right? Or how to you sync theses events (e.g. when the EventBus just notifies async and goes on with the next)?