NEventStore / Discussions

Talking about NES & vNext
0 stars 0 forks source link

Idempotence #1

Open andreabalducci opened 9 years ago

andreabalducci commented 9 years ago

Current Repository implementation checks for duplicated CommitId using an hashset on the loaded commits, so CommitId is guaranteed to be unique only for the commits loaded after a snapshot.

Most of the time this is not an issue, unless you have a very aggressive snapshot policy.

Should we move this check on the persistence level with an unique constraint? Should be an optional check?

AGiorgetti commented 9 years ago

To be absolutely sure of not processing the same commit twice the check should be moved to the presistence level.

I will not make this check optional, there's always the risk of having the same message delivered and processed twice if you have any sort of message bus / queue in you system (and if your commitId is somewhat related to a command message id that can be delivered twice).

AGiorgetti commented 9 years ago

IMO this can be made optional if you are absolutely sure that your aggregate is idempotent per se, but nothing will protect you by out of order messages that get delivered twice.

andreabalducci commented 9 years ago

Current implementation is good only for command handler retries and works only with commondomain. If we need to handle properly idempotency we need to move to the persistence level so every eventstream (with or without an aggregate) will benefit. IMHO should be opt-in, if my domain needs this functionality I can turn it on, otherwise we can allow duplicates on commit id. (could be a decorator on the persistence initialization)

mauroservienti commented 9 years ago

Isn't it wrong by design to allow duplicates on Commit``ID? If it is an identifier it must be unique, otherwise there are no guarantees.

andreabalducci commented 9 years ago

We use this as "task/command" id, in our system should be unique in a given stream (but not globally).

Param description in IEvenstStream.cs says: The value which uniquely identifies the commit. I think we should add an unique constraint on BucketId/StreamId/CommitId in every persistence layer.

AGiorgetti commented 9 years ago

I too agree that the CommitId have to be unique only inside a stream.

Let's add the constraint to the persistence layer.

alkampfergit commented 9 years ago

Also agree for the uniqueness, lets add the constraint.

mauroservienti commented 9 years ago

So, can we that stream id + commit id must be unique across the entire system?

On Fri, Jun 5, 2015 at 9:07 AM, Gian Maria notifications@github.com wrote:

Also agree for the uniqueness, lets add the constraint.

— Reply to this email directly or view it on GitHub https://github.com/NEventStore/Discussions/issues/1#issuecomment-109184006 .

Mauro Servienti Microsoft MVP - Visual C#

andreabalducci commented 9 years ago

It's Bucket / StreamId / CommitId