mikehancock / CypherNetCore

MIT License
0 stars 2 forks source link

Deal with Async in IEnlistment Notification #7

Open mtranter opened 9 years ago

mtranter commented 9 years ago

As @pvoosten has shown, we need to do something about the Async hoo-haa in IEnlistmentNotification..

I think the greater issue here is that TranscationScope() commits/rollbacks are done via the IDisposable syntax. (For the most part).

As such, I have added a branch "SyncUnitOfWork" and made the methods in ICypherUnitOfWork synchronous..

Does anyone have any major thoughts about this? I know we are trying to Async form the ground up, but I believe we will be battling the normal TransactionScope semantics if we try to do this..

ICypherUnitOfWork is internal only so this synchronous interface will not be exposed.

Mark.

pvoosten commented 9 years ago

We could use a producer-consumer pattern to perform commits. Commit commands are then sent to a blocking queue by client code and the enqueued commands are handled by a task that is started when the transaction starts. The task waits for a commit command in the queue until either such a command is received or the transaction times out. After timeout, the queue must be closed for further additions, after which a final check for a command in the queue ensures that no commit is lost.

If a commit command is sent to the queue after timeout of the transaction, the queue will already be closed for additions, so the caller will be informed that commits are no longer possible. The caller must also close the queue for further additions on sending a commit command, to avoid race conditions between callers, the queue size must be exactly one. Only the first caller will be able to add a commit command, because the consuming task will close the queue for further addition.

We can also use a CancellationToken for explicit rollbacks.

Producing a commit command can be asynchronous: fire and forget, but immediately continue. The command producer immediately knows whether the queue can accept the command, but not if the commit will be effectively performed. The transaction object can also inform observers about its state with Committed and RolledBack events, which can be (asynchronously) fired from the queue consumer task when it finishes.

All in all, if what I describe above is the direction we go into, I'm not really sure, but maybe this means that we should also rewrite the transaction manager. Transactions are supported directly in the Neo4j REST api, What would be the pros and cons of taking advantage of only the transaction support of the REST api and not System.Transactions?

Philip

mikehancock commented 9 years ago

I think the main reason for implementing System.Transactions interface was for convenience - developers don't need to do anything other than wrap a TransactionScope over CypherNet.Core to get transactional behaviour.

It may be that System.Transactions behaviour does not fit well enough into the REST API, and perhaps we need more control over how transactions are handled. The cost of not using System.Transactions is that any consuming developer would need to explicitly handle all the transactional code.

mtranter commented 9 years ago

Philip,

Under the hood we are using the REST API transactions. We are just wrapping this in the TransactionScope() semantics. Using TransactionScope also allows clients to manage transactions over multiple resources more easily.

I think a question we probably do need to ask is do we necessarily want to support this? I feel that we do, but am happy to hear other arguments..

Mark

mtranter commented 9 years ago

Sorry Mike.. Didnt see your comment

pvoosten commented 9 years ago

The current public interface (using GraphStore as an entry point) lacks explicit transaction support: image Transaction support similar as in the System.Data.Entity.Database class in Entity Framework 6 (https://msdn.microsoft.com/en-us/library/system.data.entity.database%28v=vs.113%29.aspx) may be interesting. The DbTransaction class (https://msdn.microsoft.com/en-us/library/system.data.common.dbtransaction(v=vs.113).aspx) can be an example for a custom transaction interface.