search

spring-projects / spring-integration

Spring Integration provides an extension of the Spring programming model to support the well-known Enterprise Integration Patterns (EIP)
http://projects.spring.io/spring-integration/
Apache License 2.0
1.54k stars 1.1k forks source link

Declarative transaction configuration for publish-subscribe-channel with task-executor [INT-588] #4553

Closed spring-operator closed 13 years ago

spring-operator commented 15 years ago

Grzegorz Grzybek opened INT-588 and commented

There's clean and elegant solution for \ with PollableChannel as input-channel, where one can configure task-executor attribute and \ element. This all is implemented in:

org.springframework.integration.endpoint.AbstractPollingEndpoint.Poller

Poller.run() checks whether to use task-executor, and Poller.innerPoll() checks if doPoll() may be run within TransactionTemplate.

I'm implementing simple scenario, where one "dispatcher thread" (triggered with Quartz) reads IDs and some property from DataBase, passes them through DirectChannel to some router, which (on the base of the property) passes the ID to publish-subscribe-channel. First, I've used QueueChannel with poller, but my intention was to handle the IDs immediately after the were passed to the channel.

Straight to the point - the poller has \ configuration (used to start transaction in the threads from the task-executor's pool), but EventDrivenConsumer (created for service-activator with publish-subscribe-channel as an input channel) does not.

It would be great to configure the publish-subscribe-channel with task-executor set with \ - so that the Runnables passed to task-executor where invoked within TransactionTemplate - and more precisely:

BroadcastingDispatcher.this.sendMessageToHandler(messageToSend, handler)

was invoked within TransactionTemplate just as:

org.springframework.integration.endpoint.AbstractPollingEndpoint.doPoll() is invoked within org.springframework.integration.endpoint.AbstractPollingEndpoint.Poller.innerPoll().

with best regards Grzegorz Grzybek

Affects: 1.0.1

Reference URL: http://forum.springframework.org/showthread.php?t=67897

Attachments:

2 votes, 1 watchers

spring-operator commented 15 years ago

Grzegorz Grzybek commented

Here's my patch suggestion for this issue. This is svn diff made on 1.0.3 tag:

regards Grzegorz Grzybek

spring-operator commented 15 years ago

Grzegorz Grzybek commented

Of course also:

Tests include single and multiple handlers and ignoring/don't ignoring failures

spring-operator commented 15 years ago

Grzegorz Grzybek commented

Sorry - this time with spring-eclipse code formatting convention :)

spring-operator commented 14 years ago

Mark Fisher commented

Grzegorz,

Thanks for the patch! I am wondering if we should also be adding this support for \ within a \ element for a point-to-point channel, e.g.:

<channel id="p2pChannel">
    <dispatcher task-executor="taskExecutor">
        <transactional ... />
    </dispatcher>
</channel>
spring-operator commented 14 years ago

Mark Fisher commented

Moving this to M3. We most likely need to make this work for any point-to-point channel with a "dispatcher" sub-element as well.

spring-operator commented 14 years ago

Mark Fisher commented

We need to consider this in light of the following two 2.0 alternatives and should resolve this decision for M4 if possible:

  1. gateway interceptors
  2. endpoint/handler interceptors

My main concern about the channel as an interception point for something that is essentially "around" advice is that the logical role of channel is uni-directional. Even though the dispatcher could demarcate a transaction that would in fact include the reply invocation, it is not as obvious as it would be on a handler/endpoint/gateway.

spring-operator commented 14 years ago

Mark Fisher commented

Any thoughts on that last comment I posted?

spring-operator commented 14 years ago

Grzegorz Grzybek commented

I'm a little out of touch with the current code base... Of course tx management should not be related to channel (pipe) but only to other components (filters). When I thought about this, I've realized there are three cases:

receive (from source) and send (to destination) are not about channels, but endpoints. In case of input I think it's OK to manage TX in Poller (AbstractPollingEndpoint), but the problem is with output. As I've managed to check the SI-core, there is MessageHandler (just handles the message) and AbstractMessagingGateway (which uses MessageChannelTemplate to send or receive messages).

So my mistake with the patch was to bind TX management with BroadcastingDispatcher - I think the better place is AbstractMessageHandler - maybe add TX objects to IntegrationObjectSupport (just like e.g. TaskScheduler?) - if so, then TX objects (manager, definition, template) could also be moved up from AbstractPollingEndpoint.

Also - among AbstractMessageHandler derived classes, three of them have MessageChannelTemplate object created at point of declaration - with default configuration (i.e. without TX objects). I'm aware that MessageChannelTemplate should have TX objects - it can be used "outside" Spring-Int...

So - how about refactoring TX management to three places in total:

?

regards Grzegorz Grzybek

spring-operator commented 14 years ago

Mark Fisher commented

I'm still leaning toward adding transactionality at the gateway level (similar to the Transactional Client EIP: http://enterpriseintegrationpatterns.com/TransactionalClient.html).

spring-operator commented 14 years ago

Mark Fisher commented

I'm still leaning toward adding this support at Gateways only. The gateway can now be used in a generic way (not just a proxied-interface) in order to provide request/reply semantics at any point within a message flow.

I'm moving to RC1 in the hope that we can get some feedback/opinions on that.

spring-operator commented 14 years ago

Grzegorz Grzybek commented

Isn't TransactionalClient EIP about transactions at channel level? I mean - if the Gateway must handle transaction it must not put the message into the channel (invoke send on this.messagingTemplate) in case of rollback at the sender side. Am I wrong thinking that this will be totally different tx management than in current implementation? Now transactions wrap (in case of sending a message):

This is different than TransactionalClient EIP which says "With a sender, the message isn’t “really” added to the channel until the sender commits the transaction"...

I think I'm lost now - this seemed like an easy extrapolation of si:poller's behavior...

regards Grzegorz Grzybek

spring-operator commented 14 years ago

Mark Fisher commented

The Transactional Client approach would work fine with any channel that is capable of participating in a Transaction. In our case, that would mean either a MessageStore-backed channel whose MessageStore is transactional (e.g. JdbcMessageStore) or the JMS Destination-backed channels. Other channels like our in-memory Queue-backed channel would not be transaction aware (i.e. no way to commit or rollback without building transactional awareness into those channels). OTOH, the DirectChannel is a slightly different case. It does not ever hold Messages anyways, so any commit/rollback would only apply to a Transactional resource on the other side (e.g. DirectChannel -> service-activator -> DAO).

If we were to add the transaction support directly on a \, we might need to differentiate between two options: TX that spans all subscribers vs. a per-subscriber TX. Would you agree?

spring-operator commented 14 years ago

Grzegorz Grzybek commented

So there are several scenarios... I remember the mess from Mule-ESB - you've configured transactions/synchronization in mule config and Mule did what it wanted with transactions - there were just hidden rules for tx management dependent on what kind of message exchange was performed.

I just hope Spring-Integration will have much more clear rules.

I think we need to catalog the possible scenarios (strategies?):

I think JdbcMessageStore-backed channels has also two scenarios depending on the threading. "Transactional Client EIP" would fit only for asynchronous channels - synchronous channels will "see" the message even if the TX will be rolled back...

org.springframework.integration.endpoint.AbstractPollingEndpoint.Poller.innerPoll() is another scenario (similar to MessageDispatcher with Executor).

So - the above are just my loud thoughts - I don't have so wide vision of Spring-Int as you have :)

But finally I have other thought - I know the TX Management is usually thread-based (ThreadLocals and such), but could message-based TX management be a good idea? The transaction (all these TxObjects, Connection/SessionHolders, etc.) could be bound to current message and there should be dedicated PlatformTransactionManager which won't use org.springframework.transaction.support.TransactionSynchronizationManager and its thread locals... But even if this new PlatformTransactionManager will be used in TransactionTemplate inside MessageDispatcher/Poller I have no idea how to detect "end" of transaction carried by the Message... Just a loose thought...

regards Grzegorz Grzybek

spring-operator commented 14 years ago

Mark Fisher commented

Backing up a bit... the reason we have support for transactions on the poller is that we can call receive() within that poller task against a resource that is itself transactional. OTOH, for something like a publish-subscribe-channel or a p2p channel with \ we would be adding transactions to affect all downstream components starting with that send() call. That is providing a rather wide scope for transaction demarcation without any strong reason (whereas the receive() method being included in a transaction boundary is the "strong reason" for pollers). I made a similar comment here: #5465 (and decided not to provide TX demarcation at that level).

I personally think we should recommend using the shortest-lived transactional scope as possible. Most of the times that would mean simply having standard @Transactional configuration on the individual POJOs that are being invoked behind Service Activators, etc.

So, my main point is that the poller is a special case because the receive() might perform an action that needs to be rolled back. Adding \ for publish-subscribe-channel would widen the scope unnecessarily.

I'm not going to mark this one as "Won't Fix" yet, because I realize there might be some feedback that could change my opinion, but I am considering resolving it as "Won't Fix".

spring-operator commented 14 years ago

Grzegorz Grzybek commented

Good argument! That's why I've implemented what I want with @Transactional on @ServiceActivator annotated class just before filing this issue :)

For me it is enough to let you mark it as "won't fix" - just after explaining it in reference documentation.

That'll also resolve the general problem of transactions in Spring-Int in much cleaner way than in Mule (I don't know how they do it in Mule 3, but I'm not interested in checking this out :)

Another thing - SpringInt being (IMO) the best implementation of EI Patterns should explain that it has not reference implementation of "Transactional Client EIP" because it has not the concept of transactional channels (despite of JMS backed channels) - there's no MessageChannel.commit() method. So maybe a little explanation here in reference doc?

So for me it is enough to resolve this issue by putting your explanation (on short and long transactions) in reference documentation.

regards Grzegorz Grzybek

spring-operator commented 14 years ago

Mark Fisher commented

Oleg, can you just amend the Transaction section that you recently created in the reference manual to include some detailed information/justification for this as suggested in the comments above?

Thanks, Mark

spring-operator commented 13 years ago

Oleg Zhurakousky commented

The new Transaction section was updated with explanation which was inspired by the comments from this issue.