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

Turn on optimistic concurrency by default for NHibernate sagas #649

Closed
andreasohlund opened this Issue Sep 13, 2012 · 9 comments

Comments

Projects
None yet
4 participants
@andreasohlund
Member

andreasohlund commented Sep 13, 2012

If the saga data isn't containing a Version property turn on "optimistic-all".

http://ayende.com/blog/3946/nhibernate-mapping-concurrency

This makes sure that the users isn't exposed to concurrency issues.

Given that user can be on "serializable" transaction scopes today without a Version property I don't think we can add this to 3.X in a backwards compatible way

@johnsimons

This comment has been minimized.

Show comment
Hide comment
@johnsimons

johnsimons Sep 18, 2012

Contributor

It looks like Optimistic Locking in mapping by code is not supported!
See https://nhibernate.jira.com/browse/NH-2823

Contributor

johnsimons commented Sep 18, 2012

It looks like Optimistic Locking in mapping by code is not supported!
See https://nhibernate.jira.com/browse/NH-2823

@johnsimons

This comment has been minimized.

Show comment
Hide comment
@johnsimons

johnsimons Sep 18, 2012

Contributor

The only way I think we can support this is:

  • For automatic mappings, we dump them to hbm xml and add the extra optimistic-lock="all"
  • For embedded hbm xml resources, we need to extract the xml files check if they have a version column (<version) or not and add the extra optimistic-lock="all" if required

Thoughts?

Contributor

johnsimons commented Sep 18, 2012

The only way I think we can support this is:

  • For automatic mappings, we dump them to hbm xml and add the extra optimistic-lock="all"
  • For embedded hbm xml resources, we need to extract the xml files check if they have a version column (<version) or not and add the extra optimistic-lock="all" if required

Thoughts?

@johannesg

This comment has been minimized.

Show comment
Hide comment
@johannesg

johannesg Sep 18, 2012

Contributor

Is it really necessary for hbm files? One could assume that if someone
wants to map their classes themselves then they should know what they are
doing.

2012/9/18 John Simons notifications@github.com

The only way I think we can support this is:

  • For automatic mappings, we dump them to hbm xml and add the extra
    optimistic-lock="all"
  • For embedded hbm xml resources, we need to extract the xml files
    check if they have a version column (<version) or not and add the
    extra optimistic-lock="all" if required

Thoughts?


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8639310.

Contributor

johannesg commented Sep 18, 2012

Is it really necessary for hbm files? One could assume that if someone
wants to map their classes themselves then they should know what they are
doing.

2012/9/18 John Simons notifications@github.com

The only way I think we can support this is:

  • For automatic mappings, we dump them to hbm xml and add the extra
    optimistic-lock="all"
  • For embedded hbm xml resources, we need to extract the xml files
    check if they have a version column (<version) or not and add the
    extra optimistic-lock="all" if required

Thoughts?


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8639310.

@ghost ghost assigned johnsimons Sep 19, 2012

@johnsimons

This comment has been minimized.

Show comment
Hide comment
@johnsimons

johnsimons Sep 19, 2012

Contributor

I've turned on optimistic concurrency for the auto generated saga mappings only.

As part of testing this I've noticed that when using IsolationLevel.Serializable (the default in NSB) we get a dead lock exception instead.
So I wonder if I should only do this for isolation levels < Serializable? I guess it doesn't matter.

Contributor

johnsimons commented Sep 19, 2012

I've turned on optimistic concurrency for the auto generated saga mappings only.

As part of testing this I've noticed that when using IsolationLevel.Serializable (the default in NSB) we get a dead lock exception instead.
So I wonder if I should only do this for isolation levels < Serializable? I guess it doesn't matter.

@andreasohlund

This comment has been minimized.

Show comment
Hide comment
@andreasohlund

andreasohlund Sep 19, 2012

Member

I thought we turned the isolevel down to readcommited when the NH persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels < Serializable? I
guess it doesn't matter.


Reply to this email directly or view it on
GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8679614.

Member

andreasohlund commented Sep 19, 2012

I thought we turned the isolevel down to readcommited when the NH persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels < Serializable? I
guess it doesn't matter.


Reply to this email directly or view it on
GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8679614.

@johnsimons

This comment has been minimized.

Show comment
Hide comment
@johnsimons

johnsimons Sep 19, 2012

Contributor

Not the TransactionScope isolation level

On 19 September 2012 15:55, Andreas Öhlund notifications@github.com wrote:

I thought we turned the isolevel down to readcommited when the NH
persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels < Serializable?
I
guess it doesn't matter.


Reply to this email directly or view it on
GitHub<
https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8679614>.


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8680062.

Regards
John Simons
NServiceBus

Contributor

johnsimons commented Sep 19, 2012

Not the TransactionScope isolation level

On 19 September 2012 15:55, Andreas Öhlund notifications@github.com wrote:

I thought we turned the isolevel down to readcommited when the NH
persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels < Serializable?
I
guess it doesn't matter.


Reply to this email directly or view it on
GitHub<
https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8679614>.


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8680062.

Regards
John Simons
NServiceBus

@johannesg

This comment has been minimized.

Show comment
Hide comment
@johannesg

johannesg Sep 19, 2012

Contributor

The NH persister uses the same isolation level as the ambient transaction

2012/9/19 John Simons notifications@github.com

Not the TransactionScope isolation level

On 19 September 2012 15:55, Andreas Öhlund notifications@github.com
wrote:

I thought we turned the isolevel down to readcommited when the NH
persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga
mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels <
Serializable?
I
guess it doesn't matter.


Reply to this email directly or view it on
GitHub<

https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8679614>.


Reply to this email directly or view it on GitHub<
https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8680062>.

Regards
John Simons
NServiceBus


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8680082.

Contributor

johannesg commented Sep 19, 2012

The NH persister uses the same isolation level as the ambient transaction

2012/9/19 John Simons notifications@github.com

Not the TransactionScope isolation level

On 19 September 2012 15:55, Andreas Öhlund notifications@github.com
wrote:

I thought we turned the isolevel down to readcommited when the NH
persister
is in use?(johannes)

Sent from my iPhone

On 19 sep 2012, at 06:19, John Simons notifications@github.com wrote:

I've turned on optimistic concurrency for the auto generated saga
mappings
only.

As part of testing this I've noticed that when using
IsolationLevel.Serializable (the default in NSB) we get a dead lock
exception instead.
So I wonder if I should only do this for isolation levels <
Serializable?
I
guess it doesn't matter.


Reply to this email directly or view it on
GitHub<

https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8679614>.


Reply to this email directly or view it on GitHub<
https://github.com/NServiceBus/NServiceBus/issues/649#issuecomment-8680062>.

Regards
John Simons
NServiceBus


Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/649#issuecomment-8680082.

@johnsimons

This comment has been minimized.

Show comment
Hide comment
@johnsimons

johnsimons Sep 20, 2012

Contributor

NHibernate does not support optimistic-lock="all" on derived classes :(
I guess for those the users need to specify a version field.
Thoughts?

Contributor

johnsimons commented Sep 20, 2012

NHibernate does not support optimistic-lock="all" on derived classes :(
I guess for those the users need to specify a version field.
Thoughts?

@ramonsmits

This comment has been minimized.

Show comment
Hide comment
@ramonsmits

ramonsmits Dec 7, 2012

Member

How should you do that? Just define a version property on the saga?

public virtual int Version {get;set;}
Member

ramonsmits commented Dec 7, 2012

How should you do that? Just define a version property on the saga?

public virtual int Version {get;set;}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment