Autocommit in transaction

[Daryl Stultz]

Hello,

I'm getting the following warning:

WARNING: DataSource [default] has autoCommit defaulting to true!

I noticed it a while back and wondered what sort of impending doom I
would run into. I found it. Consider the following:

Transaction tx = Ebean.beginTransaction();
Ebean.save(myBean);
Ebean.endTransaction();

The tx logs show a Rollback but the save remains in the database.
Inserting this before the save:

tx.getConnection().setAutoCommit(false);

solves the problem. The danger is it's very likely to be overlooked by
the average programmer. I've never messed with autocommit before and
I'm not inclined to change the default. I have a lot of JDBC code and
I'm transitioning from OpenJPA. It appears that OpenJPA changes
autocommit to false when a tx is started. It seems like Ebean should
do that, too. Looking at JtaTransaction line 68 we see:

if (connection.getAutoCommit()) {
connection.setAutoCommit(false);
}

But JdbcTransaction doesn't do this. I've added this

if (explicit) {
if (connection.getAutoCommit()) {
connection.setAutoCommit(false);
}
}


to JdbcTransaction line 159. It solves my problem. I'd like to keep
this change. Not sure if it's exactly as it should be. If you would
not accept it as a permanent solution, perhaps it could be
configurable (and you could suggest property names and such for
getting the setting down to JdbcTransaction). If you would not accept
it at all, please explain why.

Of course if there's a better way to do this (other than reconfigure
the driver/datasource to produce connections with autocommit false)
I'd love to hear it.

Thanks.

/Daryl

[Rob Bygrave]

I believe you'd also need to reset the connection back to autocommit=true after you have completed the transaction so that the connection is back in the same initial state prior to it going back into the pool.

So that suggests that for every explicit transaction and every non-query-only transaction you want to setAutoCommit(false) at the beginning of the transaction and setAutoCommit(true) at the end.

So then the issue becomes, what is the impact of these extra calls to setAutoCommit() ... does it send extra network packets to the DB?

Is there a reason you want your pool to be holding connections that have autoCommit=true rather than turn on autoCommit just for the transactions where you want it?  What is the motivation for using autoCommit by default?


Cheers, Rob.

[Daryl Stultz]

On Feb 14, 4:55 pm, Rob Bygrave <robin.bygr...@gmail.com> wrote:
> I believe you'd also need to reset the connection back to autocommit=true
> after you have completed the transaction so that the connection is back in
> the same initial state prior to it going back into the pool.

I saw that in your code. I ran a test with my own connection pool that
had only one connection. The autocommit state does not appear to
linger (when it comes out of the pool it's always true). Still, I
would probably want to restore it just to be safe.

> So then the issue becomes, what is the impact of these extra calls to
> setAutoCommit() ... does it send extra network packets to the DB?

Good question, one I don't know the answer to, however transactions
aren't all that common for my app.

> Is there a reason you want your pool to be holding connections that have
> autoCommit=true rather than turn on autoCommit just for the transactions
> where you want it?  What is the motivation for using autoCommit by default?

No motivation, that's just the way PostgreSQL comes by default and I
never considered it. I've never changed autocommit on a connection.
For my JDBC transactions I simply executeUpdate("begin") and either
executeUpdate("commit") or executeUpdate("rollback"). Never had a
problem with rolled back transactions "sticking".

So in this case I just don't want to change the way my JDBC or OpenJPA
code is running. Since the Ebean integration is new, I can explore new
behavior.

Perhaps instead of using Ebean directly to start/end transactions I
should funnel all transaction management through my own manager and
handle autocommit there?

What's your explanation for why JtaTransaction and JdbcTransaction
handle autocommit differently? (I don't know much about EJB.)

Thanks.

/Daryl

[Rob Bygrave]

>> Still, I would probably want to restore it just to be safe.

Yeah. I guess the point is that the extra cost of this is 2 extra calls to setAutoCommit which might be 2 extra network roundtrips - not sure.

>> just the way PostgreSQL comes by default

So the motivation for NOT using autoCommit is because it can be significantly faster with multiple statements per transaction. It effectively allows the use of the jdbc batch api which can be very common for an ORM (anytime you cascade save down a 1-M relationship for example). I don't know how autoCommit works with the jdbc batch api.

I'd be interested to know if there are any scenarios that show significant benefits to using autoCommit=true (and not calling connection.commit()).

>> why JtaTransaction and JdbcTransaction handle autocommit differently?

I wouldn't read much into that difference. I don't really expect to see JtaTransaction's connection with autoCommit=true given the expected two phase commit.

[Durchholz, Joachim]

I wouldn't bother keeping the old autocommit setting around. Just document that if the application provides a Connection to Ebean, it will have autocommit switched off. (For Connection that Ebean obtains on its own, setting autocommit to false should be one of the first actions.)

Most Java code uses autocommit=false. The only exception I ever saw was executing interactively-entered SQL, and in that case, you wouldn't want to pass the Connection to Ebean anyway.

And I wouldn't worry about server roundtrips. Optimization "just in case" is the root of all evil :-)
In this case, there are already two layers that could optimize that out: the connection pool (if it hands out wrappers around the actual Connection objects), and the JDBC driver where the optimization is really easy - just delay submitting a changed autocommit setting until the next transactional SQL statement is sent.
I'd reconsider only if concrete TCP-level logging shows that autocommit does, in fact, cause server roundtrips in relevant JDBC drivers.

[Daryl Stultz]

On Feb 15, 2:49 am, Rob Bygrave <robin.bygr...@gmail.com> wrote:
> So the motivation for NOT using autoCommit is because it can be
> significantly faster with multiple statements per transaction.

I don't doubt it's better for Ebean, I'm just saying changing it to
false for all my JDBC work would wreak havoc.

> >> why JtaTransaction and JdbcTransaction handle autocommit differently?
>
> I wouldn't read much into that difference. I don't really expect to see
> JtaTransaction's connection with autoCommit=true given the expected two
> phase commit.

I meant, why doesn't JdbcTransaction set it to false the way
JtaTransaction does?

/Daryl

[Daryl Stultz]

On Feb 15, 6:33 am, "Durchholz, Joachim" <Joachim.Durchh...@hennig-

fahrzeugteile.de> wrote:
> I wouldn't bother keeping the old autocommit setting around. Just document that if the application provides a Connection to Ebean, it will have autocommit switched off.

I changed DefaultServerFactory line 476 to:

String m = "DataSource [" + serverConfig.getName()+ "] has autoCommit
defaulting to true! Explicit transactions will set autoCommit to
false.";

> (For Connection that Ebean obtains on its own, setting autocommit to false should be one of the first actions.)

See earlier post on change I made to JdbcTransaction.

Rob, are you interested in incorporating these changes, or should I
write my own "transaction facade" that begins the transaction and sets
autocommit false?

/Daryl

[Rob Bygrave]

>> I meant, why doesn't JdbcTransaction set it to false the way JtaTransaction does?

My thinking until just now is that no one would use Ebean with autoCommit=true and so there wasn't thought to supporting it - hence I'm surprised its there in JtaTransaction.

My thinking now is that there is more to supporting autoCommit=true than just turning it off for explicit transactions (cascade save, jdbc batch, effectively 2 commits per transaction ocuring etc).

[Rob Bygrave]

>> Explicit transactions will set autoCommit to false

I don't think this is sufficient. Implicit transactions that have cascade save will also need autoCommit to be false. So every non-query transaction probably needs autoCommit set to false (which negates any possible performance benefits and increases the performance costs).

If someone doesn't use Ebean's DataSource implementation then the connections also need to be reset back.

Additional thought is that if Ebean was changed to be aware of autoCommit=true then we currently are issuing 2 commits so we really we should aim to skip connection.commit() in that case.

>> Rob, are you interested in incorporating these changes

I need to think more about the issue above. If there was a scenario where autoCommit=true really rocked in terms of performance then that would be worth looking at in terms of changing JdbcTransaction.  My current impression is extra complexity and probably extra performance cost ... which implies that we shouldn't add it into JdbcTransaction and instead use an external transaction manager (like the spring one in the ebean spring project) so that you can use Ebean with your existing code base.

[Daryl Stultz]

On Wed, Feb 15, 2012 at 6:15 PM, Rob Bygrave <robin....@gmail.com> wrote:

I don't think this is sufficient.

I need to think more about the issue above. If there was a scenario where autoCommit=true really rocked in terms of performance then that would be worth looking at in terms of changing JdbcTransaction. 

I wasn't suggesting autocommit=true was better for anything in particular, but that Ebean should switch it off if that's a better way to work. Right now it issues a warning but the behavior is actually quite bad - transactions don't protect anything.

I'm going to bail on my modification of Ebean and modify my DataSource to flip autocommit on the way out and back in.

Thanks for your input.

/Daryl

[Rob Bygrave]

>> Right now it issues a warning but the behavior is actually quite bad - transactions don't protect anything.

Quite right - it should have been FATAL given the assumptions in the code (like JdbcTransaction etc). The current situation is not good at all.

>> autocommit=true was better for anything in particular

Yup. I'm just treating it as an opportunity to think if Ebean SHOULD be trying to make use of autoCommit=true (rather than always just turn it off) because I honestly haven't put enough thought into it. Right now I think the cascade save scenario mostly puts a downer on it because to me it starts getting complicated to use it safely.

>> I'm going to bail on my modification of Ebean

Ok. Ebean will need to change so its a matter of trying to make a good decision about the change. I'll ponder the options and let you know.

The 4 options I see are:

1 - Throw a FATAL for pools using autoCommit=true ... (Not ideal, doesn't really address the issue for people who make use of autoCommit=true )

2 - Turn autoCommit=true off for the duration of any non-query-only transaction (Your original suggestion I think, we also reset autoCommit after commit/rollback as we can't assume the use of the Ebean pool which otherwise does this for us)

3 - Try to make use of autoCommit=true in some scenarios (Looks like that might not be a worth it in terms of complexity)

4 - Create an external transaction manager (seems to me this might not be a good solution if all the transactions have autoCommit turned off. More interesting if we wanted Ebean to make use of autoCommit=true for some scenario)


If I stop thinking about this now then option 3 dies and it seems to be a choice between 2 and 4. I think 2 would win in terms of simplicity.


Cheers, Rob.

[Daryl Stultz]

On Thu, Feb 16, 2012 at 3:00 PM, Rob Bygrave <robin....@gmail.com> wrote:

The 4 options I see are:

2 - Turn autoCommit=true off for the duration of any non-query-only transaction (Your original suggestion I think, we also reset autoCommit after commit/rollback as we can't assume the use of the Ebean pool which otherwise does this for us)

Not being the expert you are, I'd vote for #2. My DataSource and Connection wrapper solution is working, so I'm good for now.

Thanks.

/D

[edge]

autocommit =true can never safely be used in any write transaction
I always make sure in my datasource setup that autocommit=false e.g.
<Resource name="jdbc/demo_db" type="javax.sql.DataSource"
username="demo" password="demo"
driverClassName="com.mysql.jdbc.Driver" url="jdbc:mysql://localhost/
demo_db"
defaultAutoCommit="false" maxActive="20" reloadable="false" />

As far as I remember Oracle doesn't have an autocommit but MySQL does,
not sure about the other DBs
Anyway, #2 gets my vote but technically it could still be a problem if
the connection is used by something expecting autocommit=true during
the time Ebean as turned it off but I think that is a really far
fetched scenario


On Feb 16, 9:40 pm, Daryl Stultz <kungfumachin...@gmail.com> wrote:

[Daryl Stultz]

On Fri, Feb 17, 2012 at 3:12 AM, edge <e.mc...@imilia.com> wrote:

autocommit =true can never safely be used in any write transaction

I imagine that's an unqualified statement. PostgreSQL has autocommit and it defaults to true. My connection pool does not change it and I've been using it safely for  8 years. I simply issue a "begin" followed later by "commit". Inserts/Updates/Deletes not between "begin/commit" are committed immediately. 

What is the difference between 

autocommit=false, issuing I/U/D followed by commit

and

autocommit=true, begin, issuing I/U/D followed by commit

?

/Daryl

[edge]

On Feb 17, 2:21 pm, Daryl Stultz <kungfumachin...@gmail.com> wrote:

> On Fri, Feb 17, 2012 at 3:12 AM, edge <e.mcgr...@imilia.com> wrote:
> > autocommit =true can never safely be used in any write transaction
>
> I imagine that's an unqualified statement. PostgreSQL has autocommit and it
> defaults to true. My connection pool does not change it and I've been using
> it safely for  8 years. I simply issue a "begin" followed later by
> "commit". Inserts/Updates/Deletes not between "begin/commit" are committed
> immediately.

correct - you either have autocommit or you have a transaction that
needs a commit - you can't have both at the same time.
Autocommit=true is the default jdbc mode and is really only useful for
atomic statements where you don't need a transaction.

>
> What is the difference between
> autocommit=false, issuing I/U/D followed by commit
> and
> autocommit=true, begin, issuing I/U/D followed by commit
> ?

I think they should be equivalent

[Durchholz, Joachim]

> What is the difference between
> autocommit=false, issuing I/U/D followed by commit
> and
> autocommit=true, begin, issuing I/U/D followed by commit
> ?

The difference is in what happens after the commit.

Do you set autocommit to true after the commit? Then you might inadvertently switch the caller's setting. Not an issue if all your code is always in your by-default-true-autocommit scenario, but you don't want to wrap that code in a library and reuse in another project that has a different autocommit policy.

Do you reset autocommit to the original state after the commit? Then the autocommit setting will be correct, but the JDBC driver might issue an implicit commit even if you set autocommit from false to false (the Javadoc on setAutoCommit says "calling this will cause a commit", not "calling this will cause a commit if it actually changes the autocommit setting").

Transactional code that needs to be reusable regardless of outside autocommit settings hence probably needs to look like this:

boolean inAutocommit = conn.autoCommit ();
if (inAutocommit) {
conn.setAutoCommit (false);
}
... I/U/D code ...
if (inAutocommit) {
conn.setAutoCommit (true);
}

I consider that kind of code "bad poetry", but it does the job.
YMMV.

[Rob Bygrave]

Hi,

I have committed the change to JdbcTransaction. It turns off autoCommit for the duration of the transaction and then turn's it back on prior to putting the connection back into the pool.

ie. Option 2.


Daryl, it would be great if you can test this against you use cases if you get a chance - thanks.

Cheers, Rob.

[Daryl Stultz]

On Mon, Feb 27, 2012 at 3:11 AM, Rob Bygrave <robin....@gmail.com> wrote:

Hi,

I have committed the change to JdbcTransaction. It turns off autoCommit for the duration of the transaction and then turn's it back on prior to putting the connection back into the pool.

ie. Option 2.


Daryl, it would be great if you can test this against you use cases if you get a chance - thanks.

I've just gotten back from vacation ("holiday"). It might be a challenge for me to test this. I'm building from 2.7.4 source, not HEAD, so getting your work into my code base might be tough. I'll see if I can squeeze testing in somewhere.

Thanks.

/Daryl

[Daryl Stultz]

On Monday, February 27, 2012, Rob Bygrave wrote:

Hi,

I have committed the change to JdbcTransaction. It turns off autoCommit for the duration of the transaction and then turn's it back on prior to putting the connection back into the pool.

 

Daryl, it would be great if you can test this against you use cases if you get a chance - thanks.

I tested 2.7.5 with my unit test on this issue and it passed.

Thanks!

/Daryl 

[Daryl Stultz]

On Tuesday, June 12, 2012 7:35:20 AM UTC-4, Daryl Stultz wrote:

Daryl, it would be great if you can test this against you use cases if you get a chance - thanks.

I tested 2.7.5 with my unit test on this issue and it passed.

I found a new scenario that doesn't work. I start with a JDBC connection and open a transaction. I make a change in JDBC, then run a read-only query (SqlQuery in this case), then commit the transaction. When the Ebean query exits, it appears the flipping of the auto commit rolls back the transaction, the change made in JDBC does not stick. I can well imagine this is an outside use case, I don't like mixing JDBC with Ebean, especially with transactions, but I've got a big project and I'm converting things little by little. I've rolled back the change for 379 to make it work for me. If the feature is still deemed useful to some, perhaps I need a way to disable it.

/Daryl

[Vaughn Butt]

I've found another problem with the 379 change to JdbcConnection.

This new line in /ebean/trunk/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java, revision:1384

this.autoCommit = connection.getAutoCommit();

assumes that the connection is not null.

However, the constructor for JTATransaction invokes the JdbcTransaction with a null connection and then sets up the connection.

It appears to be possible to avoid the NullPointerException and retain the original functionality (as intended by the 379 change) with the following changes:

• Make JdbcTransaction.autoCommit not final
• Add the following null check around the autoCommit assignments to the JdbcTransaction constructor:

      if (connection!=null) {

     this.autoCommit = connection.getAutoCommit();

          if (this.autoCommit) {

              connection.setAutoCommit(false);

            }

      }

• Add the setting of this.autoCommit to the JtaTransaction constructor:

            this.autoCommit = connection.getAutoCommit();

            if (connection.getAutoCommit()) {

                connection.setAutoCommit(false);

            }

I can supply a patch file if you want.