eligosource / eventsourced

A library for building reliable, scalable and distributed event-sourced applications in Scala
Apache License 2.0
828 stars 98 forks source link

Add read only mode to journals #137

Closed patelh closed 10 years ago

patelh commented 10 years ago

Added readOnly as a property to journal props and updated tests to test read only mode.

krasserm commented 10 years ago

Hi Hiral,

thanks for your contribution. Why don't you simply decorate the journal actors (during instantiation with actorOf) with a special actor (say ReadOnlyFacade) if readOnly is true. The ReadOnlyFacade actor simply drops all write commands and only forwards read commands to the journals actor. This would have the following advantages:

Maybe I'm missing something but wouldn't this dramatically simplify things?

Cheers, Martin

patelh commented 10 years ago

Sure, we can try using a mixin at construction time. I will refactor to do this. However, using a base type which provides the write path methods may be a better separation of concerns then just having a facade on top of any Actor which intercepts all messages and redirects write specific commands to do nothing.

I also think we should keep the Writer trait introduced in SynchronousWriteReplaySupport which is just inspired by the AsynchronousWriteReplaySupport Writer (a better separation of concerns).

krasserm commented 10 years ago

I didn't mean using a mixin at construction time, rather a separate actor (read-only facade) that has a reference to a journal actor. This facade actor only forwards read commands.

krasserm commented 10 years ago

I also think we should keep the Writer trait introduced in SynchronousWriteReplaySupport which is just inspired by the AsynchronousWriteReplaySupport Writer (a better separation of concerns).

This will break existing external journal implementations.

patelh commented 10 years ago

Hm, using another actor to essentially filter on write specific commands is less efficient then building it right into the write path at construction time of journal. But yes it is less invasive but feels more of a bandage or duct tape on top.

Existing external journal implementations that use the SynchronousWriteReplaySupport would break, you're right. Maybe we can create a new trait which has similar abstractions to the AsynchronousWriteReplaySupport and create an implementation of SynchronousWriteReplaySupport on top of the new trait without changing method signatures. Then it would be easier to deprecate SynchronousWriteReplaySupport in favor of the new trait with better separation of concerns. What do you think?

patelh commented 10 years ago

Nevermind, I think this is a moot point since all this will be in akka-persistence. I'll work on a less invasive patch.

krasserm commented 10 years ago

Hm, using another actor to essentially filter on write specific commands is less efficient then building it right into the write path at construction time of journal.

You can ignore that overhead relative to the much higher IO overhead

But yes it is less invasive but feels more of a bandage or duct tape on top.

Why? This is idiomatic actor design.

Then it would be easier to deprecate SynchronousWriteReplaySupport in favor of the new trait with better separation of concerns.

What concerns do you want to separate and what is the advantage over the existing implementation?

krasserm commented 10 years ago

Nevermind, I think this is a moot point since all this will be in akka-persistence.

In akka-persistence, there will be read-only processors (=read views) for which you can create snapshots

krasserm commented 10 years ago

I'll work on a less invasive patch.

Great, thanks!

patelh commented 10 years ago

What concerns do you want to separate and what is the advantage over the existing implementation?

I was mostly thinking of the Replayer, Writer and Snapshotter that are in the Async version.

patelh commented 10 years ago

Let me know if you'd like any other changes.

krasserm commented 10 years ago

Looks good, thanks a lot for your contribution @patelh

krasserm commented 10 years ago

Jars are now published to the Eligotech repo.

patelh commented 10 years ago

Awesome, thanks!