+1
Original Transaction class also has many tightly-coupled assumptions about
the underlying data source, lock master. Developers are usually lost on
when and where they should use @DB, for nested transactions, it does not
really work as expected.
Kelven
On 10/9/13 10:38 AM, "Chiradeep Vittal" <chiradeep.vit...@citrix.com>
wrote:
>+1 to option B (for a lot of the reasons enunciated by Darren).
>Also, let's get this in right away so that by 1/31/2014 we are confident
>about the change and fixed any bugs uncovered by the new scheme.
>
>On 10/9/13 10:29 AM, "Darren Shepherd" <darren.s.sheph...@gmail.com>
>wrote:
>
>>Pedro,
>>
>>From a high level I think we'd probably agree. Generally I feel an
>>IaaS platform is largely a metadata management framework that stores
>>the "desired" state of the infrastructure and then pro-actively tries
>>to reconcile the desired state with reality. So failures should be
>>recovered from easily as inconsistency will be discovered and
>>reconciled. Having sad that, ACS is not at all like that. It is very
>>task oriented. Hopefully I/we/everyone can change that, its a huge
>>concern of mine. The general approach in ACS I see is do task X and
>>hopefully it works. If it doesn't work, well hopefully we didn't
>>leave things in an inconsistent state. If we find it does leave
>>things in an inconsistent state, write a cleanup thread to fix bad
>>things in bad states....
>>
>>Regarding TX specifically. This is a huge topic. I really don't know
>>where to start. I have so many complaints with the data access in
>>ACS. There's what I'd like to see, but its so far from what it really
>>is. Instead I'll address specifically your question.
>>
>>I wish we were doing transaction per API, but I don't think that was
>>ever a consideration. I do think the sync portion of API commands
>>should be wrapped in a single transaction. I really think the
>>original intention of the Transaction framework was to assist in
>>cleaning up resources that people always forget to close. I think
>>that is mostly it.
>>
>>The general guidelines of how I'd like transactions to work would be
>>
>>1) Synchronous portions of API commands are wrapped in a single
>>transaction. Transaction propagation capability from spring tx can
>>then handle nesting transaction as more complicated transaction
>>management may be need in certain places.
>>
>>2) Async jobs that run in a background threads should do small fine
>>grained transaction management. Ideally no transactions.
>>Transactions should not be used as a locking mechanism.
>>
>>Having said that, there are currently so many technical issues in
>>getting to that. For example, with point 1, because IPC/MessageBus
>>and EventBus were added recently, that makes it difficult to do 1.
>>The problem is that you can't send a message while a DB tx is open
>>because the reciever may get the message before the commit. So
>>messaging frameworks have to be written in consideration of the
>>transaction management. Not saying you need to do complex XA style
>>transactions, there's simpler ways to do that. So regarding points 1
>>and 2 I said. That's what I'd like to see, but I know its a long road
>>to that.
>>
>>Option B is really about introducing an API that will eventually serve
>>as a lightweight wrapper around Spring TX. In the short term, if I do
>>option B, the implementation of the code will still be the custom ACS
>>TX mgmt. So across modules, its sorta kinda works but not really.
>>But if I do the second step of replacing custom ACS TX impl with
>>Spring TX, it will follow how Spring TX works. If we have Sprint TX
>>we can then leverage the transaction propagation features of it to
>>more sanely handle transaction nesting.
>>
>>I feel I went a bit the weeds with that response, but maybe something
>>in there made sense.
>>
>>Darren
>>
>>On Wed, Oct 9, 2013 at 9:31 AM, Pedro Roque Marques
>><pedro.r.marq...@gmail.com> wrote:
>>> Darren,
>>> My assumption when I tried to make sense of the transaction code is
>>>that the underlying motivation is that the code is trying to create a
>>>transaction per API call and then allow multiple modules to implement
>>>that API call...
>>> i.e. the intent is do use a bit of what i would call a "web-server
>>>logic"...
>>>
>>> 1. API call starts.
>>> 2. Module X starts transaction...
>>> 3. Module Y does some other changes in the DB...
>>> 4. Either the API call completes successfully or not... commit or error
>>>back to the user.
>>>
>>> I suspect that this was probably the starting point... but it doesn't
>>>really work as i describe above. Often when the plugin i'm working on
>>>screws up (or XenServer is misconfigured) one ends up with DB objects in
>>>inconsistent state.
>>>
>>> I suspect that the DB Transaction design needs to include what is the
>>>methodology for the design of the management server.
>>>
>>> In an ideal world, i would say that API calls just check authorization
>>>and quotas and should store the intent of the management server to reach
>>>the desired state. State machines that can then deal with transient
>>>failures should then attempt to move the state of the system to the
>>>state intended by the user. That however doesn't seem to reflect the
>>>current state of the management server.
>>>
>>> I may be completely wrong... Can you give an example in proposal B of
>>>how a transaction would span multiple modules of code ?
>>>
>>> Pedro.
>>>
>>> On Oct 9, 2013, at 1:44 AM, Darren Shepherd wrote:
>>>
>>>> Okay, please read this all, this is important... I want you all to
>>>> know that its personally important to me to attempt to get rid of ACS
>>>> custom stuff and introduce patterns, frameworks, libraries, etc that I
>>>> feel are more consistent with modern Java development and are
>>>> understood by a wider audience. This is one of the basic reasons I
>>>> started the spring-modularization branch. I just want to be able to
>>>> leverage Spring in a sane way. The current implementation in ACS is
>>>> backwards and broken and abuses Spring to the point that leveraging
>>>> Spring isn't really all that possible.
>>>>
>>>> So while I did the Spring work, I also started laying the ground work
>>>> to get rid of the ACS custom transaction management. The custom DAO
>>>> framework and the corresponding transaction management has been a huge
>>>> barrier to me extending ACS in the past. When you look at how you are
>>>> supposed to access the database, it's all very custom and what I feel
>>>> isn't really all that straight forward. I was debugging an issue
>>>> today and figured out there is a huge bug in what I've done and that
>>>> has lead me down this rabbit hole of what the correct solution is.
>>>> Additionally ACS custom transaction mgmt is done in a way that
>>>> basically breaks Spring too.
>>>>
>>>> At some point on the mailing list there was a small discussion about
>>>> removing the @DB interceptor. The @DB interceptor does txn.open() and
>>>> txn.close() around a method. If a method forgets to commit or
>>>> rollback the txn, txn.close() will rollback the transaction for the
>>>> method. So the general idea of the change was to instead move that
>>>> logic to the bottom of the call stack. The assumption being that the
>>>> @DB code was just an additional check to ensure the programmer didn't
>>>> forget something and we could instead just do that once at the bottom
>>>> of the stack. Oh how wrong I was.
>>>>
>>>> The problem is that developers have relied on the @DB interceptor to
>>>> handle rollback for them. So you see the following code quite a bit
>>>>
>>>> txn.start()
>>>> ...
>>>> txn.commit()
>>>>
>>>> And there is no sign of a rollback anywhere. So the rollback will
>>>> happen if some exception is thrown. By moving the @DB logic to the
>>>> bottom of stack what happens is the transaction is not rolled back
>>>> when the developer thought it would and madness ensues. So that
>>>> change was bad. So what to do.... Here's my totally bias description
>>>> of solutions:
>>>>
>>>> Option A or "Custom Forever!": Go back to custom ACS AOP and the @DB.
>>>> This is what one would think is the simplest and safest solution.
>>>> We'll it ain't really. Here's the killer problem, besides that fact
>>>> that it makes me feel very sad inside, the current rollback behavior
>>>> is broken in certain spots in ACS. While investigating possible
>>>> solutions I started looking at all the places that do programmatic txn
>>>> management. It's important to realize that the txn framework only
>>>> works properly if the method in which you do txn.start() has @DB on
>>>> it. There is a java assert in currentTxn() that attempts to make sure
>>>> that @DB is there. But.... nobody runs with asserts on. So there are
>>>> places in ACS where transactions are started and no @DB is there, but
>>>> it happens to work because some method in the stack has @DB. So to
>>>> properly go back to option A we really need to fix all places that
>>>> don't have @DB, plus make sure people always run with asserts on. And
>>>> then give up making the ACS world a better place and just do things
>>>> how we always have...
>>>>
>>>> Option B or "Progress is Good": The current transaction management
>>>> approach (especially rollback) doesn't match how the majority of
>>>> frameworks out there work. This option is to change the Transaction
>>>> class API to be more consistent with standard frameworks out there. I
>>>> propose the following APIs (if you know Spring TX mgmt, this will look
>>>> familiar)
>>>>
>>>> 1) remove start(), commit(), rollback() - The easiest way to ensure we
>>>> up date everything properly is to break the API and fix everything
>>>> that is broken (about 433 places)
>>>> 2) Add execute(TransactionCallback) where TransactionCallback has one
>>>> method doInTransaction(). For somebody to run a transaction you would
>>>> need to do
>>>>
>>>> txn.execute(new TransactionCallback() {
>>>> Object doInTransaction() {
>>>> // do stuff
>>>> }
>>>> })
>>>> 3) add "Object startTransaction()," commit(Object), and
>>>> rollback(Object) - These methods are for callers who really really
>>>> want to do thing programmatically. To run a transaction you would do
>>>>
>>>> Object status = txn.startTransaction()
>>>> try {
>>>> //.. do stuff
>>>> txn.commit(status)
>>>> } catch (Exception e) {
>>>> txn.rollback(status)
>>>> }
>>>>
>>>> I'm perfectly willing to go and change all the code for this. It will
>>>> just take a couple hours or so. Option B is purposely almost exactly
>>>> like Spring PlatformTransactionManager. The reason being if we switch
>>>> all the code to this style, we can later drop the implementation of
>>>> Transaction and move to 100% fully Spring TX managed.
>>>>
>>>> Just as a final point, every custom approach or framework we have adds
>>>> a barrier to people extending ACS and additionally puts more burden on
>>>> the ACS community as that is more code we have to support. If
>>>> somebody today wants to know how DB transaction propagation works,
>>>> there's zero documentation on it and probably 2 people who know how it
>>>> works. If we use a standard framework then somebody can just refer to
>>>> that frameworks documentation, community, or stackexchange.
>>>>
>>>> So either option we can do and I'm opening this up to the community to
>>>> decide. If we go with Option A, a small portion of me will die
>>>> inside, but so be it.
>>>>
>>>> Darren
>>>
>
