cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Darren Shepherd <darren.s.sheph...@gmail.com>
Subject [DISCUSS] Transaction Hell
Date Wed, 09 Oct 2013 08:44:48 GMT
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