Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Revise eventsourcing api #3773
Revise eventsourcing api #3773
Conversation
…on options for application, generate warnings in log by default
|
@sebastianburckhardt Shouldn't this be merged in v2.0? This is even small, but a breaking change in the API. |
|
@imagyar I agree with that suggestion. I am still waiting for @jason-bragg to take a look, he has been very busy with other work. |
|
We were hoping to fit this in for 2.0, but it doesn't look like it's going to make it. Post 2.0 we'll revisit prioritization and see where this fits in. I fully understand that this PR and other Event Sourcing changes have been pushed off for months now. I can't commit to a timeframe at this point, only that we'll revisit the prioritization post 2.0 and should be in a better position to set expectations then. |
| /// <returns></returns> | ||
| protected virtual void RaiseEvent<TEvent>(TEvent @event) | ||
| /// <returns>a task that completes when the event has been confirmed</returns> | ||
| protected async Task RaiseEvent<TEvent>(TEvent @event) |
rikbosch
Nov 28, 2018
Contributor
Migrations of old code will be error-prone when this method is not awaited. Maybe it is better to rename the method, so code breaks at compile time instead of runtime.
Migrations of old code will be error-prone when this method is not awaited. Maybe it is better to rename the method, so code breaks at compile time instead of runtime.
sebastianburckhardt
Jul 26, 2019
Author
Contributor
Good point. I think for this particular method, we can just use RaiseEventAsync, which is a better name anyway.
Good point. I think for this particular method, we can just use RaiseEventAsync, which is a better name anyway.
|
Looking at this PR again after a very long time. There's a number of breaking changes here. I don't see a way to take this without breaking all existing users of |
|
As far as I remember, the only breaking changes are name changes that do not affect functionality. Those were suggested by @jdom and @ReubenBond when we had a meeting to discuss this, a long time ago. I guess at that time, introducing a breaking change seemed not a major issue, and we were trying to improve the names slightly. At this point of time, I would agree that changing the names seems neither necessary nor a good idea. I think it makes sense to keep the current names and not change the API. I can revise this accordingly. |
|
It's been too long to remember, but I think I recall concluding that JournaledGrain wasn't really usable as is, so there were probably no consumers of it. Perhaps we should ask. Every time I heard people doing ES in Orleans, they had their own infrastructure for it |
|
I don't think "unusable" is the right word, as people are apparently using it. It just appears they implemented their own LogConsistencyProvider since the included providers were not quite right for their purposes. We knew that all along. @jkonecki and I implemented a better provider (for EventStore) in a PR but it all got stuck for some reason, other things being considered more important by the Orleans team at the time (e.g. the whole 2.0 overhaul). If there is interest in continuing this line of work it can still happen. |
|
I would be happy to work on event sourcing support - I have a library that
implements event store on top of CosmosDB which will be easy to integrate
(currently using it directly from grains, OnActivate)
…On Thu, 25 Jul 2019, 21:21 Sebastian Burckhardt, ***@***.***> wrote:
I don't think "unusable" is the right word, as people are apparently using
it. It just appears they implemented their own LogConsistencyProvider since
the included providers were not quite right for their purposes. We knew
that all along.
@jkonecki <https://github.com/jkonecki> and I implemented a better
provider (for EventStore) in a PR but it all got stuck for some reason,
other things being considered more important by the Orleans team at the
time (e.g. the whole 2.0 overhaul). If there is interest in continuing this
line of work it can still happen.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3773?email_source=notifications&email_token=AANRNDYRTFHIJ2V7UPG5K4DQBIDN7A5CNFSM4EHPFV32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22VDRA#issuecomment-515199428>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANRND5EEHHPGGAGAT3RS4LQBIDN7ANCNFSM4EHPFV3Q>
.
|
|
Good to hear that you could take it forward. I assume that with the revamp we did in 2.0 and with GrainActivationContext, all of this can actually be implemented without any changes to the core runtime, correct? The Orleans team is no longer modifying the core when it can be avoided. So for example in the Transactions work -which requires lots of hooks into the grain activation lifecycle- they were able to implement it on top of the current extension points, as would any 3rd party customer would be able to do so without forking the code. |
|
I don't think I'll be able to do everything on my own: I'm not familiar
with core changes as I'm currently using 1.5.x and I haven't looked how
transactions are implemented.
…On Thu, 25 Jul 2019, 22:09 Julian Dominguez, ***@***.***> wrote:
Good to hear that you could take it forward. I assume that with the revamp
we did in 2.0 and with GrainActivationContext, all of this can actually be
implemented without any changes to the core runtime, correct? The Orleans
team is no longer modifying the core when it can be avoided. So for example
in the Transactions work -which requires lots of hooks into the grain
activation lifecycle- they were able to implement it on top of the current
extension points, as would any 3rd party customer would be able to do so
without forking the code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3773?email_source=notifications&email_token=AANRND5OU24ZMF32NUKHHJLQBII7RA5CNFSM4EHPFV32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22Y7KA#issuecomment-515215272>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANRND3PV2ALP4HKOQHWESDQBII7RANCNFSM4EHPFV3Q>
.
|
|
We just talked with @sebastianburckhardt and agreed to revamp this PR and add @jkonecki Your contribution will be very welcome. We can help with putting it in the right context in the 2.0/3.0 world. It doesn't have to be confined by |
|
Closing as this got much outdated. |
One commit covers several small revisions to the event sourcing API.
RaiseEventfor the most common expected case, i.e. conditional events that are not retried on races.LostRaceException) if a raised event loses a race to the shared logThe other commit covers fixes and improvements to the retry logic and error diagnostics
Once this is done I will need to update the documentation accordingly as well.