Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idempotence #1

Open
andreabalducci opened this issue May 29, 2015 · 9 comments
Open

Idempotence #1

andreabalducci opened this issue May 29, 2015 · 9 comments

Comments

@andreabalducci
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

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
Copy link
Member

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

Let's add the constraint to the persistence layer.

@alkampfergit
Copy link

Also agree for the uniqueness, lets add the constraint.

@mauroservienti
Copy link

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
#1 (comment)
.

Mauro Servienti
Microsoft MVP - Visual C#

@andreabalducci
Copy link
Member Author

It's Bucket / StreamId / CommitId

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants