Shuttle / shuttle-esb

Documentation for the Shuttle.Esb free open-source .NET/Core enterprise service bus.
http://shuttle.github.io/shuttle-esb/
127 stars 44 forks source link

MsmqQueue currently ignores TransactionScope #49

Closed ltvan closed 8 years ago

ltvan commented 8 years ago

Currently MsmqQueue always sent messages with MessageQueueTransactionType.Single. This defeats the purpose of TransactionScope. I suggest that it should be like this:

    if (Transaction.Current != null)
    {
        queue.Send(sendMessage, MessageQueueTransactionType.Automatic);
    }
    else
    {
        queue.Send(sendMessage, MessageQueueTransactionType.Single);
    }

BTW, I would like to ask if I should post issue like this here or in the shuttle-esb-msmq.

eben-roux commented 8 years ago

BTW, I would like to ask if I should post issue like this here or in the shuttle-esb-msmq.

It really does not matter :)

Although msmq related issues would fit better be in the shuttle-esb-msmq issues section, the reasoning for my answer is related to the shuttle-esb-core.

From version 3 of shuttle there has been a move away from distributed transactions since not all queuing mechanisms support them. This is why the msmq queue is not enlisted in the ambient transaction. If you have a look at the older code you will find something along the same implementation as you've described.

The TransactionScope should remain useful since all database interactions would be included. Databases are more likely to implement a resource manager for distributed transactions in the event that you do have more than one in use within the same transaction. Since it is entirely possible to switch off the TransactionScope usage for an endpoint you could even handle transactions yourself, although doing so could cause problems with usage of an IdempotenceService that makes use of a database (as the SqlServer one does).

Is this causing you any grief that we could perhaps try to resolve?

ltvan commented 8 years ago

I think that if user don't want to use distributed transaction, then user will have to disable the TransactionScope. If user enables TransactionScope, the MSMQ should be enlisted too.

For the IdempotenceService, if the TransactionScope is enabled, it should not create another transaction or the transaction will be failed if MSDTC is not enabled. The TransactionScope works just fine as long as we use SqlServer 2008, all connection strings are the same, and do not create any sub transaction. (http://stackoverflow.com/questions/1690892/transactionscope-automatically-escalating-to-msdtc-on-some-machines)

If user don't want to use TransactionScope and control the transaction themselves, the SqlServer version of IdempotenceService should provide some callback or provider to create/get user-defined ambient transaction.

eben-roux commented 8 years ago

Enlisting msmq into the ambient transaction requires a different sequence of events and quite a bit of different handling compared to the 'acknowledgement' model used currently. Unfortunately it isn't quite as simple as enlisting the queue into the transaction.

I'm afraid that doing so would require a different queue interface and different pipeline handling so I would be loathe to go back to that mechanism as the current handling seems to work quite OK.

Is there any particular reason why you cannot use it as is?

ltvan commented 8 years ago

http://docs.particular.net/nservicebus/outbox/

I need to make sure the message are sent once the application transaction complete. Currently, if transaction failed, the message are still sent. This is not acceptable, especially if we are sending a SendMailCommand message.

ShuttleESB doesn't have the Outbox mechanism like NServiceBus. I want to mimic the behavior by using the TransactionScope, at least at this moment. I'm using SqlServer 2008 with only one database per solution so it won't be escalated to MSDTC.

ltvan commented 8 years ago

Could you please explain more about acknowledgement model that you are applying? I don't understand why it could not be just as simple as enlisting the queue into the transaction.

eben-roux commented 8 years ago

Shuttle has an outbox, but its purpose is somewhat different from the outbox NServiceBus uses. From what I can tell from the NServiceBus outbox documentation the shuttle IIdempotenceService does exactly the same thing :)

Not all queuing mechanisms support distributed transactions (such as RabbitMQ). This is why I decided to move away from DTC. Here is some opinions:

As Udi states, moving away from distributed transactions means a bit more complexity but it certainly is possible.

NServiceBus has a history of tight coupling with msmq and since msmq supports DTC all is well. But believe me when I tell you that those transactions do not necessarily make your life that much easier as a service bus developer. For instance, let's assume you allow for 1 retry and an error occurs while handling your message in an enlisted DTC transaction including msmq and a sql server database. The first retry is fine since you simply roll back the transaction and the message is returned to the queue. However, the second failure means moving the message to an error queue and this error queue should not be enlisted is the same transaction as before since the first has to be rolled back to ignore your database changes but the error has to be enqueued. However, rolling back means the original message also returns to the inbox queue. So how do we move the message? Some nifty footwork is required but, once again, it is possible. I'm not going to bore you with those details though:)

So the way the acknowledgement model roughly works, without an IIdempotenceService implementation but with a TransactionScope, is as follows:

So if you are using the above then you will most definitely have messages immediately sent and processed.

However, when using an IdempotenceService implementation with a TransactionScope (without a TransactionScope here would defeat the purpose) the processing differs slightly:

For the above the IIdempontenceService implementation has to support transactions/DTC to be useful. Also, should an error occur while sending the deferred messages the original message becomes available again and the pipeline re-runs. Since it has been handled it will not call the handler but only continue the sending of the deferred messages. These are sent one-by-one and immediately removed from the deferred message table (NServiceBus outbox) once sent. If, by some edge case, a duplicate is sent the IdempotenceService on the receiver endpoint will prevent duplicated processing.

Shuttle has, therefore, been on an at-least-once message sending mechanism from version shuttle 3.0.0 versus the only-once message sending provided by distributed transaction (for queues that support DTC) and that was available prior to shuttle 3.0.0.

So shuttle does indeed have a mechanism similar to the NServiceBus outbox. Chances are we probably had it first :)

I hope that helps.

ltvan commented 8 years ago

I thought that IdempotenceService is just for deduplicate receiving message so I didn't pay attention at the first time. I will try it. I think that you should update your document here. A side note is that I'm quite anti-video-tutorial since it takes time. So if you mentioned this in your video, I'm sorry that I didn't watch your video. Reading is always my favorite, it's much faster.

I also have some thinking about this IdempotenceService. In the future, I'd like to separate this feature from message handler, so I can have an "outbox" everywhere. I can start a transaction, doing something and sending the message, commit transaction and message sent, or if something happens, the message can be resent later. The dedup processing is still wrapping the message handler as it is. By this way, I can also control the isolation level per transaction instead of sticking at only one isolation level per whole application.

Regarding enlising Msmq in TransactionScope, I see your point. It could be complicated here. I need time to think more about this, although I still see that we can just enlist the sending message, while the other operation we can keep it out of TransactionScope. As I guess from your code, the ReceiveExceptionObserver is executed after the TransactionScope disposed.

eben-roux commented 8 years ago

I have created a couple of guides that broadly contain the same content as the associated video. I initially intended to remove the videos but some folks like them so I'll leave them for now. I actually have a mention of the deferred messages here:

However, I agree that I need to flesh it out somewhat.

If I understand your wanting to use the IdempotenceService outside of the receiving pipeline I'm afraid that it may be near impossible. I've actually given this some thought when I started the at-least-once delivery implementation.

Let's assume that your sending application performs some database interaction and also sends a message. That message can be deferred. However, should an error occur everything gets rolled back. All is still OK with this. Now, let's assume that the transaction is committed. What will perform the sending of the deferred messages? In the event that an error occurs while sending these what would carry on sending them later? Since many endpoints will have an idempotence service instance we also need to ensure that, should the idempotence service perform such sending, it is locked to prevent duplicates.

What I eventually came up with is to always ensure you have a single message entry point that would start off proceedings. So instead of performing those database operations and then sending a message of two, rather create a message that would have that same processing performed in a handler. This way the shuttle code can be kept the way it is and it will be less code to maintain and also less error prone.

Please let me know what you think, and whether may reasoning makes sense :)

ltvan commented 8 years ago

We can wrap around the transaction and do the sending once the transaction complete. In case of system crashes after the transaction complete but before the sending happens, we can eventually check the deferred messages and send them, for example when the system resumes.

In the event that error occurs while sending messages, this should be similar to your 'outbox' feature, except that you will have to retry sending the messages later instead of rely on Msmq.

What I eventually came up with is to always ensure you have a single message entry point that would start off proceedings. So instead of performing those database operations and then sending a message of two, rather create a message that would have that same processing performed in a handler. This way the shuttle code can be kept the way it is and it will be less code to maintain and also less error prone.

There are many case that I need to have this 'outbox' feature. For example in a legacy code, refactoring them could be a huge effort. Transaction may be failed after triggering an email message. Other scenarios such as user submitting a request and require a response immediately (all changes locally, no need for message queue) but still involve publishing some domain events for the other system to sync the data.

ltvan commented 8 years ago

Actually I see that you have OutboxPipeline for sending the message in outbox. What I suggest is the same.

ltvan commented 8 years ago

BTW, what happens if I use SqlServer queue for Outbox and wrap the sending command in a TransactionScope? Is there any risk? Have you tested this scenario?

eben-roux commented 8 years ago

I must admit that you have spotted an interesting usage for the outbox :)

I have just tested the scenario and it seems to work fine:

App.config:

<connectionStrings>
    <add
        name="shuttle"
        connectionString="Data Source=.;Initial Catalog=shuttle;Integrated Security=SSPI;" 
        providerName="System.Data.SqlClient" />
</connectionStrings>

<serviceBus>
    <messageRoutes>
        <messageRoute uri="rabbitmq://localhost/requestresponse-server-inbox-work-transient?durable=false&amp;persistent=false">
            <add specification="StartsWith" value="RequestResponse"/>
        </messageRoute>
    </messageRoutes>
    <inbox workQueueUri="rabbitmq://localhost/requestresponse-client-inbox-work-transient?durable=false&amp;persistent=false" errorQueueUri="rabbitmq://localhost/shuttle-samples-error"/>
    <outbox
         workQueueUri="sql://shuttle/outbox"
         errorQueueUri="rabbitmq://localhost/shuttle-samples-error" />
</serviceBus>

Client code:

using (var tx = bus.Configuration.TransactionScopeFactory.Create(null))
{
    Console.WriteLine("Message (id: {0}) sent.  Text: {1}", bus.Send(command, c =>
    {
        c.WithCorrelationId("correlation-id");
        c.Headers.Add(new TransportHeader {Key = "header1", Value = "value1"});
        c.Headers.Add(new TransportHeader {Key = "header2", Value = "value2"});
    }).MessageId, command.Text);

    // tx.Complete();
}

The message is not sent if the tx.complete is not called. I am pretty impressed by this! Well spotted!

Unless I am missing something this mechanism should cover your use case. For some odd reason I pass a PipelineEvent to the TrasnactionScopeFactory.Create method. I do not use it so I'll investigate it a bit and remove it should it not required (see issue https://github.com/Shuttle/shuttle-esb-core/issues/4).

ltvan commented 8 years ago

I tried and it still has something that needs to enhance. If I open a connection, save something, close the connection, send a message, then it works fine. However it's never easy in real life. In real application, the send message command is run within another db connection scope (I'm using Entity Framework), then the SqlQueue open another connection and bum, exception tells that MSDTC is not enabled because there are 2 connections are opened at the same time.

I have to rewrite the SqlQueueFactory and a lot of database things in order to inject the ambient connection into the SqlQueue. That's just some hack, I'm in the middle of implementing it and it does not work right now. Hope that you have time to improve your core data abstraction layer to open for customization like this. For example IDbConnectionLocator for ambient connection instead of couple with your own DataConnectionCache.

eben-roux commented 8 years ago

I'll have to take another look at my data access structures to see how to accommodate this.

However, in the meantime I did get by the escalation using the following code:

var threadStaticDatabaseConnectionCache = new ThreadStaticDatabaseConnectionCache();
var dataSource = new DataSource("shuttle", new SqlDbDataParameterFactory());

using (var tx = bus.Configuration.TransactionScopeFactory.Create(null))
using (var connection = new SqlConnection("Data Source=.;Initial Catalog=shuttle;Integrated Security=SSPI;"))
using (new DatabaseConnection(dataSource, connection, new DbCommandFactory(), threadStaticDatabaseConnectionCache)) // <-- underlying connection opened here
{
    using (var command = connection.CreateCommand())
    {
        command.CommandType = CommandType.Text;
        command.CommandText = string.Format("insert into SomeTable (SomeValue) values ('{0}');", Guid.NewGuid());
        command.ExecuteNonQuery();
    }

    Console.WriteLine("Message (id: {0}) sent.  Text: {1}", bus.Send(message, c =>
    {
        c.WithCorrelationId("correlation-id");
        c.Headers.Add(new TransportHeader { Key = "header1", Value = "value1" });
        c.Headers.Add(new TransportHeader { Key = "header2", Value = "value2" });
    }).MessageId, message.Text);

    tx.Complete();
}

The reason it works is because internally the DatabaseGateway retrieves the connection from the ThreadStaticDatabaseConnectionCache using the DataSource name. Since we are registering this before a new connection is created the existing connection will be used. It is important to note that it is not the best idea to use ThreadStaticDatabaseConnectionCache in a web environment since worker threads may move around (well, for IIS) while processing a request. There is an HTTP version for that. But then again, the calls here are going to be in the same method and not anywhere along the ASP.NET pipeline.

By the way, I must mention that had you enlisted a msmq queue the transaction would immediately escalated to a distributed transaction using DTC since 2 resource managers would be involved :)

It seems you have done some seriously deep digging into the shuttle stuff :) --- thanks for making the effort.

ltvan commented 8 years ago

I suggest you to read this post: http://mehdi.me/ambient-dbcontext-in-ef6/

You can apply the same method to your data access layer. Using CallContext should resolve the worker thread issue in IIS. I also learn the idea of IDbConnectionLocator from this post. You can also abstract to Scope pattern and apply to any kind of ambient context.

By the way, I must mention that had you enlisted a msmq queue the transaction would immediately escalated to a distributed transaction using DTC since 2 resource managers would be involved :)

I thought so at the first time but when I tried it, it wasn't escalated.

eben-roux commented 8 years ago

I have added a SqlQueueDatabaseConnectionFactory. The following is the best I think I can do:

using (var bus = ServiceBus.Create())
{
    var databaseConnectionFactory = (SqlQueueDatabaseConnectionFactory)SqlQueueDatabaseConnectionFactory.Default();

    bus.Configuration.QueueManager.RegisterQueueFactory(
        new SqlQueueFactory(ScriptProvider.Default(),
        databaseConnectionFactory,
        DatabaseGateway.Default()));

    bus.Start();

    while (true)
    {
        var input = Console.ReadLine();

        if (!string.IsNullOrEmpty(input) && input.Equals("exit", StringComparison.OrdinalIgnoreCase))
        {
            return;
        }

        var message = new ReverseTextCommand
        {
            Text = Guid.NewGuid().ToString().Substring(0, 5)
        };

        using (var tx = bus.Configuration.TransactionScopeFactory.Create())
        using (var connection = new SqlConnection("Data Source=.;Initial Catalog=shuttle;Integrated Security=SSPI;"))
        {
            connection.Open();

            databaseConnectionFactory.AssignThreadDbConnection(connection);

            using (var command = connection.CreateCommand())
            {
                command.CommandType = CommandType.Text;
                command.CommandText = string.Format("insert into SomeTable (SomeValue) values ('{0}');", Guid.NewGuid());
                command.ExecuteNonQuery();
            }

            Console.WriteLine("Message (id: {0}) sent.  Text: {1}", bus.Send(message).MessageId, message.Text);

            tx.Complete();
        }
    }
}

I will create some documentation around this at some point: https://github.com/Shuttle/shuttle-esb/issues/50

In short you should switch off queue factory scanning and only add the other queues you'll be using (so exclude sql server):

<queueFactories scan="false">
    <add type="Shuttle.ESB.Msmq.MsmqQueueFactory, Shuttle.ESB.Msmq" />
    <add type="Shuttle.ESB.RabbitMQ.RabbitMQQueueFactory, Shuttle.ESB.RabbitMQ" />
</queueFactories>

Then register the SqlQueueDatabaseConnectionFactory before starting the bus instance. Within you transaction you can assign the connection for the thread to the connection factory. It will use that connection for the thread internally. I hope that makes sense.

I actually tested the distributed transaction for msmq/sql server in the same TrasnactionScope and it seems to require DTC. If you think about it DTC certainly should be involved as you would be coordinating transactions for msmq and sql server. That screams distributed transaction :)

ltvan commented 8 years ago

Thanks for your support. I will try it. By the way, for the future, if you want to move away from DTC, you will have to allow injecting not only connection but also transaction.

Regarding to the Msmq and DTC, I will retry it later. Maybe I missed something. I used LocalDb and Entity Framework in my testing.

ltvan commented 8 years ago

Ah, I review your changes and there should be a bug. Your DatabaseConnection are supposed to be disposed at the end of each use. Then the SqlConnection will also be disposed after sending a message. I suggest that you have a flag to indicate the internal/external owned connection.

ltvan commented 8 years ago

I could be great if you provide a way to inject IDatabaseConnectionCache so I can add my own CallContextConnectionCache in order to ensure it work in ASP.NET pipeline.

Thanks.

ltvan commented 8 years ago

Ah sorry, I see it now. Just pass it through the DatabaseGateway.

eben-roux commented 8 years ago

I have been giving this some more thought and I am not entirely happy with the current solution. It feels somewhat like a hack.

I am considering adding another interface for queues called something like IQueueEnqueue that will look something like this:

void Enqueue(Guid messageId, Stream stream, object parameters);

The TransportMessageConfigurator could be renamed to something more generic so that one could configure a QueueParameters object and then the DispatchTransportMessageObserver could safe cast the relevant queue to IQueueEnqueue if there are parameters and call through the new interface. Having QueueParameters and not being able to cast to IQueueEnqueue would result in an exception.

Then one could have some container object that contains arbitrary queue specific information that could be passed to the queue implementation. In this way one could pass the IDbConnection and transaction across for consumption.

While I am at it I am considering changing the whole data layer database connection caching business. Having these thread static bits are troublesome. It may be better just passing the connection data along in the call since currently I am passing a DataSource. May as well replace the DataSource with the connection :)

I have tried to keep the queue implementation as independent as possible but I guess there may be some use in passing data to it somehow. If, at a later stage, passing of data is required for the other methods one could implement specific interfaces for those also.

Let me know what you think.

ltvan commented 8 years ago

I think that adding an IQueueEnqueue just make your solution unnecessary complex. Actually I haven't used your SqlQueueDatabaseConnectionFactory since it makes my code fragment. However, as you open your SqlQueueFactory to pass in the other dependencies, I can implement my own IDatabaseConnectionFactory and I think that's fine and open enough for different type of data access layer. There are 2 things that you can enhance your code:

  1. If you add a flag to DatabaseConnection so it won't dispose the connection it does not own, then user can save from duplicate your DatabaseConnection.
  2. If you convert your ThreadStaticDatabaseConnectionCache to using CallContext, then people don't have to create another one for their ASP.NET application.

Regarding restructuring data access layer, passing either connection or datasource along the call stack will pollute your methods. Try my suggestion above by combining ambient connection context and connection locator (http://mehdi.me/ambient-dbcontext-in-ef6/), your code should be cleaner and loose couple.

eben-roux commented 8 years ago

I'll take another look at your link. I actually have something along that line for web/wcf environments:

https://github.com/Shuttle/shuttle-core-data-http/blob/master/Shuttle.Core.Data.Http/ContextDatabaseConnectionCache.cs

As for the disposing bit, in my current structure you'll see that when creating a new DatabaseConnection it is wrapped in an ExistingDatabaseConnection that isn't disposed.

However, I want to move away from the ThreadStatic stuff. Passing a connection should be fine and relying on the ADO.NET factories and low-level interfaces certainly makes sense.

I am no fan of ORMs. I have also removed my unit of work implementation since there isn't all that much use for it. Using DDD one shouldn't be updating multiple ARs if it can be avoided and even in the event that one does need to the usual transactional support should be fine.

ltvan commented 8 years ago

One more thing, if you allow to add external connection, make sure that you also allow to add Transaction as well. In case that we don't want to use TransactionScope, we can still use Outbox with existing transaction.

ltvan commented 8 years ago
    public void AssignThreadDbConnection(IDbConnection dbConnection)
    {
        _dbConnection = dbConnection;
    }

    public IDatabaseConnection Create(DataSource dataSource)
    {
        return _dbConnection != null
            ? new DatabaseConnection(dataSource, _dbConnection, _dbCommandFactory, _databaseConnectionCache)
            : _databaseConnectionFactory.Create(dataSource);
    }

You are returning a DatabaseConnection with the external dbConnection assigned and it will be disposed and removed from the IDatabaseConnectionCache.

Please see my suggest changes in my fork: ltvan@ee2d73653c8642c6f261cd7f61e734c7ec372af5

ltvan commented 8 years ago

Regarding ORM things, although the article I referred is about Entity Framework, you should be able to apply the pattern to any kind of ambient context, not just EF. I used to apply this pattern for my DAL with just ADO.NET.

ltvan commented 8 years ago

One more thing: currently IdempotenceService still relies on TransactionScope. If you can introduce a ConnectionContextScope and TransactionContextScope then you can remove the dependency to TransactionScope entirely.

eben-roux commented 8 years ago

I'm sold.

Do you think both a ConnectionContextScope1 *and*TransactionContextScope` is required?

Would the transaction not be tightly coupled to the connection anyway?

It seems as though ASP.NET still requires a different implementation:

http://blogs.msdn.com/b/ploeh/archive/2007/07/28/callcontextsvsaspnet.aspx

eben-roux commented 8 years ago

I have cleaned up the shuttle-core-data somewhat but it turns out it is very much what I had :)

I was thinking about this some more and I cannot see how one would get past the distributed transactions in most cases. From the NServiceBus Outbox documentation (http://docs.particular.net/nservicebus/outbox/) they actually mention the exact same caveats as I did in this thread"

Remember that each service endpoint *instance*\ would need its own outbox and each logical** service endpoint would need its own idempotence store.

So all-in-all not much is gained by this exercise other than some cleaning up and racking my brain about these things again :)

ltvan commented 8 years ago

Do you think both a ConnectionContextScope and TransactionContextScope is required?

Would the transaction not be tightly coupled to the connection anyway?

I don't have enough experience to answer thoroughly this question. In the past I have both classes in my projects. In a recent project, I have only one DataContextScope that has both methods Create and CreateWithTransaction as same as the DbContextScope in the post I mentioned, and it still works fine for now. But the idea that the transaction is tightly couple to the connection makes me feel something not flexible enough.

Think about a multi-tenants application that you have to manage multiple databases in one application, you could have many connections at the same time, and you can create only one TransactionContextScope at the top of the call stack, and all different inner ConnectionContextScope will automatically "enlist" in that transaction. However, it looks like I'm making my design overkill.

It seems as though ASP.NET still requires a different implementation:

http://blogs.msdn.com/b/ploeh/archive/2007/07/28/callcontextsvsaspnet.aspx

I read this before. The reason that I go with CallContext for the db connection and transaction is that:

  1. CallContext supports parallel tasks where each task has its own context, while with HttpContext we can only have one current context at a time. It also supports async in ASP.NET, where thread-switching also occurs after awaiting, but this still keeps the CallContext in sync.
  2. Background thread cannot access HttpContext because it isn't initiated from a HTTP request.
  3. The issue with ASP.NET thread-switching across page events can be overcome by some rules: don't use your scopes across the page life cycle events, i.e. you shouldn't begin the scope at BeginRequest and end it at EndRequest. Keep the db connection as short as possible.

Shuttle ESB is supposed to run on background threads, so it must use CallContext for storing connection and transaction.

For other kind of context that you have only one context per whole request or session, you can use some kind of abstract context loader and implement the concrete context loader based on type of application platform.

ltvan commented 8 years ago
  • NSB: Both the business data and deduplication data need to share the same database
    • The chances of this being true is slim as many database owners are not going to allow this.

If they don't allow database sharing then they have to accept the TransactionScope, but if I allow it then I can use my own transaction context. Once you can remove the dependency to TransactionScope, people can choose to either use or not use it depends on their situation. In fact, currently I can use your Outbox outside of message handler without the need of TransactionScope. :-) However, inside message handler, I still have to keep TransactionScope if I want to use your IdempotenceService.

  • NSB: The Outbox is bypassed when sending messages "from outside" via the IBus interface (not from a message handler).
    • This is where I suggested using a single message to get into the bus infrastructure. We can include the outbox in the transaction scope but it will probably result in a distributed transaction.

NSB bypasses Outbox when sending messages from outside a message handler because they don't have mechanism to resend it later. But here you had it already :-) . Try to think outside of the box and you can do what people can't.

The next step is that you should separate the Dedup and Outbox process so that they don't have to couple to each other and increase performance. Right now a message has to go through IdempotenceService deferred queue and Outbox queue before reaching the target. I'm taking this trade-off for the reliability of the whole system. If you separate the Dedup process from IdempotenceService, then we can combine Dedup and Outbox to create the Idempotence ability with less message round trip while we can still use Outbox feature separately.

eben-roux commented 8 years ago

But the idea that the transaction is tightly couple to the connection makes me feel something not flexible enough.

Transactions are started specifically from a connection object so, to me, it feels quite natural. Transactions cannot incorporate more than one transaction. That is what DTC and 2PC is for.

CallContext is pretty nifty and I'll definitely be using that.

Background thread cannot access HttpContext because it isn't initiated from a HTTP request.

This is where my current ContextDatabaseConnectionCache is used to use what is available and have a fallback in a web/wcf environment. My refactored implementation will remain, at least logically, the same.

Shuttle ESB is supposed to run on background threads, so it must use CallContext for storing connection and transaction.

The processing threads do run as threads separate to the ASP.NET pipeline. However, when sending in, say, a controller (MVC) then the normal ASP.NET worker thread will be used.

I truly do not believe that relying on TransactionScope is at all a bad idea. It is part of the core and has been developed specifically to deal with many of the problems raised. All transactions will only be promoted if necessary and when required.

The next step is that you should separate the Dedup and Outbox process so that they don't have to couple to each other and increase performance

The outbox and de-duplication are currently separate. NServiceBus seems to have these related. The outbox in shuttle is used solely for delivery.

If you separate the Dedup process from IdempotenceService, then we can combine Dedup and Outbox to create the Idempotence ability with less message round trip while we can still use Outbox feature separately.

The whole idea behind the idempotence service is to facilitate idempotence by de-duplication. I do not think it would make too much sense in splitting that out. The idempotence service would not be doing much then. one could argue that a sql outbox could always be used but that was never the intention. For instance, RabbitMQ does not have the concept of an outgoing queue as msmq has. In msmq if the recipient queue is unavailable the message is placed in the outgoing queue until is can be delivered. In such as case an outbox adds no value (other than the bonus implementation that you now spotted). With RabbitMQ, however, it would be of use as the recipient queue may not be available and storing the message locally (a la store-and-forward) would make sense so that the sending can still succeed.

I hope that makes sense. Once I am done with the data access refactoring I'll let you know and you can have a look and see what you can manage to get going with that. If we can improve anything that would be great but at this stage it doesn't seem as though there are going to be any big wins from any such refactoring.

eben-roux commented 8 years ago

If there are any more questions please feel free to open another issue :)