cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [DISCUSS] Transaction Hell
Date Wed, 09 Oct 2013 11:46:12 GMT
Darren,

Thank you for the great investigative work. As for the assert, I think
that if no one else at least Jenkins should run with asserts on, so we
can see if a commit breaks something.

As for the framework, I think your proposed solution sound good. I am
not a big fan of spring. It does some things very good but it tries to
do to much. If you say it is a good framework for transaction
management I have to trust you as I didn't use that for more then five
years. I would certainly not let you in the pit as sole owner of the
change though as I have been in a short struggle with what you are
trying to solve as well. I would like to hear from the originators of
the present transaction management though.

regards,
Daan

On Wed, Oct 9, 2013 at 11:38 AM, Frankie Onuonga <frankie@angani.co> wrote:
> Hi Darren,
> Greetings from Nairobi.
>
> First, let me start by saying that you for the email .
> I personally think the options were explained very well.
>
> Now on a serious note, I think that it would be good practice to ensure that we go with
a standardized method of doing things.
> I therefore support the option to go with a framework.
> It should make management of code easier.
> Future development should also be much better.
>
> Kind Regards,
>
>
> Sent from my Windows Phone
> ________________________________
> From: Darren Shepherd<mailto:darren.s.shepherd@gmail.com>
> Sent: 10/9/2013 11:46 AM
> To: dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>
> Subject: [DISCUSS] Transaction Hell
>
> 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

Mime
View raw message