NServiceBus/NServiceBus — comment on commit 44f79a38a54aebfcfe7d3bc070fd90cce523b884
File: src/impl/SagaPersisters/RavenSagaPersister/NServiceBus.SagaPersisters.Raven/RavenSagaPersister.cs line 59
By johannesg:

Well, remember that the Unique attribute was not part of the core api at
all but of the NH persister only until Jonathan moved it a few days ago.
So, IMHO, the api would not be changed. The Key attribute would be part of
the Raven persister while the Unique attribute would still be part of the
NH persister. If you, while upgrading to 3.0, decide to change persister
then you would have to take this into consideration but changing database
paradigm is not something you should take easy anyway.

Anyway, the main reason I started thinking this way are my concerns of the
current implementation of unique properties. I might be wrong but it seems
that there is a possibility of a race condition. If 2 messages come in at
the same time with the same id mapped to a unique property, then the raven
persister would not detect it and create 2 sagas instead of letting the
other one fail, since the uniqueness is not enforced by Raven.

2012/1/15 Andreas hlund <
reply@reply.github.com
>

> Agree with Udi that the api need to stay the same. But you're right in
> that we should try to use raven to our advantage in order to create a
> performant saga storage. I see nothing wrong in actually using the document
> id to enforce uniqueness since that is the "Raven" way of doing it.
>
> What is your thoughts Jonathan?
>
>
> On Sun, Jan 15, 2012 at 10:17 AM, Udi Dahan <
> reply@reply.github.com
> > wrote:
>
> > I wouldn't want our top level API to be influenced by lower level
> concerns
> > - definitely not at this stage of releasing 3.0. I suppose it's something
> > that *could* be considered for the next version, but I'm going to need
> some
> > convincing.
> >
> > Udi
> >
> >
> > -----Original Message-----
> > From: Johannes Gustafsson [mailto:reply@reply.github.com]
> > Sent: Sunday, January 15, 2012 12:40 AM
> > To: Udi Dahan
> > Subject: Re: [NServiceBus] Changed RavenSagaPersister to handle
> > UniqueAttribute properties using raven queries rather than a separate
> > document (44f79a3)
> >
> > I got another idea: whenever I set a property as unique it is usually
> only
> > one and also that property serves as the natural id of the saga, for
> > example a customer id or invoice id. Also, almost all messages are mapped
> > to that particular id instead of the saga id. So, since RavenDB supports
> > any kind of string as id, why not use that property as the actual
> document
> > id? The saga id in this case is usually never used and if it were used
> > anyway (in a ISagaMessage) it can be queried using a lucene index. Since
> > the saga id is a guid and guaranteed to be unique the persister don't
> even
> > need to validate it's uniqueness.
> >
> > So, what we need is a [Key] attribute instead of a [Unique]. The default
> > behavior will still be to use the saga id as document id. But if there is
> > one (and only one) property with a [Key] attribute this property will be
> > the document id instead.
> >
> > This way we can use Raven:s id-handling to our advantage instead of
> trying
> > to gain uniqueness in ways that Raven wasn't designed to handle.
> >
> > What do you think? If you like this I can probably scoop up a proof on
> > concept to see if it works...
> >
> > /Johannes
> >
> > 2012/1/14 Andreas hlund <
> > reply@reply.github.com
> > >
> >
> > > Yes, lets go with that!
> > >
> > > On Fri, Jan 13, 2012 at 11:22 PM, Johannes Gustafsson <
> > > reply@reply.github.com
> > > > wrote:
> > >
> > > > NH supports multiple attributes. In NH:s case it's really simple, it
> > > > just adds a unique constraint for every column marked as unique.
> > > >
> > > > Although I have been thinking of use cases with multiple unique
> > > properties,
> > > > I don't have a problem supporting only one. On the other hand, I see
> > > > no real reason for the core to restrict it just because some saga
> > > > persisters cant support it.
> > > >
> > > > How about letting each saga persister implementation decide? Maybe
> > > > some implementations simply cant support unique properties. In that
> > > > case, just let them throw an exception during startup if someone
> > > > tries to do it anyway.
> > > >
> > > >
> > > >
> > > > 2012/1/13 Andreas hlund <
> > > > reply@reply.github.com
> > > > >
> > > >
> > > > > Johannes: Is the NH saga persister supporting multiple [Unique]
> > > > attributes?
> > > > >
> > > > > All: Given that its very rare to need multiple unique properties.
> > > Should
> > > > we
> > > > > restrict users to only support one. This makes it easier to
> > > > > implement
> > > new
> > > > > saga storage's. Users needing more than one should can be guided
> > > > > to set their own constraints in manually?
> > > > >
> > > > > On Fri, Jan 13, 2012 at 2:32 AM, Jonathan Matheus <
> > > > > reply@reply.github.com
> > > > > > wrote:
> > > > >
> > > > > > No, I was saying that we can use session.Delete. We can also
> > > > > > support multiple properties pretty easily. I can do both
> > > > > > tomorrow
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Jan 12, 2012, at 3:51 PM, Andreas
> > > > > > hlund
> > > > > wrote:
> > > > > >
> > > > > > > So you're saying that we can support multiple unique
> properties?
> > > > > > >
> > > > > > > On Thu, Jan 12, 2012 at 6:18 PM, Jonathan Matheus <
> > > > > > > reply@reply.github.com
> > > > > > >> wrote:
> > > > > > >
> > > > > > >> We can now that we're using a UOW. We previously opted not to
> > > > > > >> rely
> > > > on
> > > > > > >> Raven change tracking, but recently changed our minds. I'll
> > > > > > >> change
> > > > it.
> > > > > > >>
> > > > > > >> ---
> > > > > > >> Reply to this email directly or view it on GitHub:
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-861627
> > > > > > >>
> > > > > > >
> > > > > > > ---
> > > > > > > Reply to this email directly or view it on GitHub:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-862610
> > > > > >
> > > > > > ---
> > > > > > Reply to this email directly or view it on GitHub:
> > > > > >
> > > > > >
> > > > >
> > > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-863209
> > > > > >
> > > > >
> > > > > ---
> > > > > Reply to this email directly or view it on GitHub:
> > > > >
> > > > >
> > > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-865707
> > > > >
> > > >
> > > > ---
> > > > Reply to this email directly or view it on GitHub:
> > > >
> > > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-866345
> > > >
> > >
> > > ---
> > > Reply to this email directly or view it on GitHub:
> > >
> > > https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d
> > > 3bc070fd90cce523b884#commitcomment-866478
> > >
> >
> > ---
> > Reply to this email directly or view it on GitHub:
> >
> >
> https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d3bc070fd90cce523b884#commitcomment-867852
> > -----
> > No virus found in this message.
> > Checked by AVG - www.avg.com
> > Version: 2012.0.1901 / Virus Database: 2109/4742 - Release Date: 01/14/12
> >
> > ---
> > Reply to this email directly or view it on GitHub:
> >
> >
> https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d3bc070fd90cce523b884#commitcomment-868384
> >
>
> ---
> Reply to this email directly or view it on GitHub:
>
> https://github.com/NServiceBus/NServiceBus/commit/44f79a38a54aebfcfe7d3bc070fd90cce523b884#commitcomment-868480
>