GeoTools

Default autoCommit causes DB2 JDBC to fail on conn.close()

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 2.0.0
  • Fix Version/s: 2.7.6
  • Component/s: jdbc-postgis plugin
  • Labels:
    None
  • Environment:
    Windows XP

Description

If the query to be executed does not perform a write operation, Query::executeQuery issues conn.setAutoCommit(false). This appears to be connected to a need by PostGIS for this.

With the DB2 JDBC, this causes an exception when the connection is closed but a commit has not performed. With the call to setAutoCommit(false), when the result set and statement are closed, no commit is performed and the subsequent attempt to close the connection fails.

The logic in executeQuery should probably delegate the value of autoCommit to be set to the appropriate DataStore.

This is a minor problem now but it will be a major problem if the DB2 JDBC is supported.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andrea Aime added a comment -
Christian, I don't think this issue still affects DB2-ng, but can you confirm?
Show
Andrea Aime added a comment - Christian, I don't think this issue still affects DB2-ng, but can you confirm?
Hide
Permalink
Christian Mueller added a comment -
Hmmm, I take a look into executeQuery and have seen the setAutoCommit call is still there.

Why do we need this ?

Could this happen to a connection passed by the user (GEOT-2009) ?.
We cannot modify the autocommit value for a connection passed by the user, this is not nice :-)

As far as I can remember, this was one of the problems I had when trying to use the jdbc2 feature store in an EJB Container. A JDBC connection enlisted in the global transaction manager cannot have autocommit true --> Exception.

For the close statement , the behavior differs depended on the connection type (XA or not XA). Having autocommit false and closing without commit/rollback can result in a warning and a rollback or in an exception.

I would vote for removing the autocommit call in executeQuery and add a pulbic API "setAutoCommit(boolean)".












Show
Christian Mueller added a comment - Hmmm, I take a look into executeQuery and have seen the setAutoCommit call is still there. Why do we need this ? Could this happen to a connection passed by the user ( GEOT-2009 ) ?. We cannot modify the autocommit value for a connection passed by the user, this is not nice :-) As far as I can remember, this was one of the problems I had when trying to use the jdbc2 feature store in an EJB Container. A JDBC connection enlisted in the global transaction manager cannot have autocommit true --> Exception. For the close statement , the behavior differs depended on the connection type (XA or not XA). Having autocommit false and closing without commit/rollback can result in a warning and a rollback or in an exception. I would vote for removing the autocommit call in executeQuery and add a pulbic API "setAutoCommit(boolean)".
Hide
Permalink
Andrea Aime added a comment -
If we're talking about the JDBC-NG code I guess the code you're referring to is in JDBCFeatureSource line 534:

{code}
// this allows PostGIS to page the results and respect the fetch size
            if(getState().getTransaction() == Transaction.AUTO_COMMIT) {
                cx.setAutoCommit(false);
            }
{code}

The comment says it all, we do that or PostGIS won't respect the fetch size and return all the data in a single shot. I guess we should roll a method to prepare the connection before reads are performed so that PostGIS can be treated specially by its dialect?
Note that the connection pools will normally issue a rollback when the connection is returned to the pool (with a performance hit, sigh)
Show
Andrea Aime added a comment - If we're talking about the JDBC-NG code I guess the code you're referring to is in JDBCFeatureSource line 534: {code} // this allows PostGIS to page the results and respect the fetch size             if(getState().getTransaction() == Transaction.AUTO_COMMIT) {                 cx.setAutoCommit(false);             } {code} The comment says it all, we do that or PostGIS won't respect the fetch size and return all the data in a single shot. I guess we should roll a method to prepare the connection before reads are performed so that PostGIS can be treated specially by its dialect? Note that the connection pools will normally issue a rollback when the connection is returned to the pool (with a performance hit, sigh)
Hide
Permalink
Andrea Aime added a comment -
And yeah, we should pay attention to the case of a user provided connection, in that case the dialect should probably not touch it.

Ah, mind another thing, in some dialects (H2) the connection need to be "initialized" to be usable (mostly because the database, in that case, need to be checked and eventually spatialized), that is another issue for externally provided connections
Show
Andrea Aime added a comment - And yeah, we should pay attention to the case of a user provided connection, in that case the dialect should probably not touch it. Ah, mind another thing, in some dialects (H2) the connection need to be "initialized" to be usable (mostly because the database, in that case, need to be checked and eventually spatialized), that is another issue for externally provided connections
Hide
Permalink
Christian Mueller added a comment -
yep, we are talking about JDBCFeatureSource line 534.

At the end of the day, we will need JDBC* classes getting connections as arguments and another set of classes having the current functionality. The second set uses the first set. I am thinking about this since more then one year but had not time to ride the attack.

For the moment, refactoring the db specific code into the dialect class should be the way to go.



Show
Christian Mueller added a comment - yep, we are talking about JDBCFeatureSource line 534. At the end of the day, we will need JDBC* classes getting connections as arguments and another set of classes having the current functionality. The second set uses the first set. I am thinking about this since more then one year but had not time to ride the attack. For the moment, refactoring the db specific code into the dialect class should be the way to go.
Hide
Permalink
Andrea Aime added a comment -
Roger about moving stuff to dialects.
As for J2EE, I have a crazy idea. What about you provide to the data store a special data source that can either:
* look up for the "current" connection some way in the execution context (thinking a thread local here)
* grab a new connection from a delegate data source if none is available (say you want the feature type list but at the moment you're not involved in a XA transaction)

As for avoidng changes to the connection, probably the best way is to play like the connection pools: we wrap the user provided connection with our own custom proxy that disallows any direct call to commit/rollback/close. End of story, unless you manually unwrap the connection (which is sometime necessary, for example when dealing with Oracle STRUCT objects) there is no way one of the three commands can be called.
Show
Andrea Aime added a comment - Roger about moving stuff to dialects. As for J2EE, I have a crazy idea. What about you provide to the data store a special data source that can either: * look up for the "current" connection some way in the execution context (thinking a thread local here) * grab a new connection from a delegate data source if none is available (say you want the feature type list but at the moment you're not involved in a XA transaction) As for avoidng changes to the connection, probably the best way is to play like the connection pools: we wrap the user provided connection with our own custom proxy that disallows any direct call to commit/rollback/close. End of story, unless you manually unwrap the connection (which is sometime necessary, for example when dealing with Oracle STRUCT objects) there is no way one of the three commands can be called.
Hide
Permalink
Christian Mueller added a comment -
Using a TLS for the connection is not a bad idea. Since a thread could need multiple connections, the thread local var should be a hashmap with weak keys, having the datastroe as key and the connection as value. +1 for this one.

About the wrapper. Normally, I would say that a developer should know what she/he is doing, but looking in the future and expecting more dialects/developers, we should protect this architecture.
A connection wrapper throwing an exception when calling "dangerous" methods makes me smile, but has an educational effect. Beyond commit,rollback and close, setAutoCommit is the next candidate. I think there are some more. Again a +1

I think this is a big step for apps modifying geometries and classical data in one transaction and it should work with XA transactions too. At the moment I cannot use a JNDI XA Datasource from Websphere because of the implicit result set close when next() returns false. But this is another issue for me after we succeeded with this one.
Show
Christian Mueller added a comment - Using a TLS for the connection is not a bad idea. Since a thread could need multiple connections, the thread local var should be a hashmap with weak keys, having the datastroe as key and the connection as value. +1 for this one. About the wrapper. Normally, I would say that a developer should know what she/he is doing, but looking in the future and expecting more dialects/developers, we should protect this architecture. A connection wrapper throwing an exception when calling "dangerous" methods makes me smile, but has an educational effect. Beyond commit,rollback and close, setAutoCommit is the next candidate. I think there are some more. Again a +1 I think this is a big step for apps modifying geometries and classical data in one transaction and it should work with XA transactions too. At the moment I cannot use a JNDI XA Datasource from Websphere because of the implicit result set close when next() returns false. But this is another issue for me after we succeeded with this one.
Hide
Permalink
Andrea Aime added a comment -
As far as I remember that is because of PostGIS, if we don't do that the entire result set is send to the driver in one shot, causing an instant OOM on larger data sets.
Show
Andrea Aime added a comment - As far as I remember that is because of PostGIS, if we don't do that the entire result set is send to the driver in one shot, causing an instant OOM on larger data sets.
Hide
Permalink
Andrea Aime added a comment -
Yep, confirmed, here is a blog of just a few months ago about this issue: http://www.chrisjordan.ca/node/34
Show
Andrea Aime added a comment - Yep, confirmed, here is a blog of just a few months ago about this issue: http://www.chrisjordan.ca/node/34
Hide
Permalink
Andrea Aime added a comment -
Did not notice the issue was already discussed
Show
Andrea Aime added a comment - Did not notice the issue was already discussed

People

  • Assignee:
    Christian Mueller
    Reporter:
    David Adler
Vote (0)
Watch (2)

Dates

  • Created:
    Updated: