implement IPersistTimeoutsV2 interface

[timbussmann]

Who's affected

You might be affected if you are using one of the following transport configurations:

• RabbitMQ transport
• Azure transport
• MSMQ or SQL transport with distributed transactions (DTC) disabled.

and you are using timeouts or deferred messages (e.g. Bus.Defer(...))

Symptoms

When experiencing connectivity issues with the transport there's a chance that a due timeout/deferred message is lost. Therefore the message will never arrive.

What to do if you are affected

We highly recommend to update to the latest patch versions of your NServiceBus and optional persistence and transport packages. For more details see issue #2885

Connects to Particular/NServiceBus#2885

[timbussmann]

@Particular/nservicebus nhibernate experts please review

[ramonsmits]

As discussed during the review:

• Normal operation is peek, send, delete. During peek we can do a row lock so that the delete will always succeed. Currently the peek and delete are executed in the same transaction when using connection sharing between sql transport and nhibernate persistence. Maybe this can potentially happen when running with multiple instances are fetching timeouts as these instance are competing on the timeout queue. (ISession.Get<T>(object, LockMode.Upgrade)
• The entity mapping is duplicate except for one property. Unknown why this is or what behavioral change this will have. Maybe refactor to one mapping due to the number of properties and using object initializer instead of a constructor which can potentially result in missing properties if the object is extended in the future.

[timbussmann]

@ramonsmits just had a look at the LockMode.Upgrade when peeking but I get some unwanted behavior in the unit test. If you have two concurrent transactions creating an update lock, can they deadlock each other?

[ramonsmits]

No, that should not be the case as it then does a SELECT FOR UPDATE (rowlock), that should not result in a deadlock, it can only result in a sequential operation. 2nd transaction will have to wait before the first completes(commit or abort) but the 1st does not have to wait for the 2nd because it already acquired a row write lock.

[timbussmann]

@ramonsmits never mind, It seems to be an issue with SQLite, same code works on SQLExpress.

[timbussmann]

@ramonsmits please review the latest commit, I changed the code according to your points:

• using upgrade lock for peek. This should only improve scenarios when using dtc for now, since as we saw, this version doesn't yet support connection sharing. So the only case where that lock may be held until the TryRemove is for ambient transactions.
• extracted the entity mapping so both Peek and TryRemove behave the same.

[ramonsmits]

Why the method name change? Test is unaltered?

[timbussmann]

to distinct between GetNextChunk and Peek tests

[timbussmann]

@Particular/nservicebus @ramonsmits any further concerns or can we merge this?

[andreasohlund]

Looks good to me