[TAP] Finish fixing the transaction management bug. (5ac8f1fb) · Commits · Sonia Zorba / vollt

src/tap/ADQLExecutor.java

+1 −4

Original line number	Diff line number	Diff line
		@@ -417,11 +417,8 @@ public class ADQLExecutor {
		logger.logTAP(LogLevel.WARNING, report, "END_EXEC", "Can not drop the uploaded tables from the database!", e);
		}

		// Free the connection (so that giving it back to a pool, if any, otherwise, just free resources):
		// Free the connection (so that giving it back to a pool if any, otherwise just free resources):
		if (dbConn != null){
		// be sure no query is still processing and that any open transaction is rollbacked and ended:
		dbConn.cancel(true);
		// free the connection:
		service.getFactory().freeConnection(dbConn);
		dbConn = null;
		}

src/tap/config/ConfigurableTAPFactory.java

+2 −2

Original line number	Diff line number	Diff line
		@@ -281,8 +281,8 @@ public class ConfigurableTAPFactory extends AbstractTAPFactory {
		@Override
		public void freeConnection(DBConnection conn){
		try{
		// Cancel any possible query that could be running:
		conn.cancel(true);
		// End properly any query that is not yet stopped and cleaned (i.e. no more transaction opened):
		conn.endQuery();
		// Close the connection (if a connection pool is used, the connection is not really closed but is freed and kept in the pool for further usage):
		((JDBCConnection)conn).getInnerConnection().close();
		}catch(SQLException se){

src/tap/data/ResultSetTableIterator.java

+177 −98

File changed.

Preview size limit exceeded, changes collapsed.

src/tap/db/DBConnection.java

+60 −11

Original line number	Diff line number	Diff line
		@@ -16,17 +16,20 @@ package tap.db;
		* You should have received a copy of the GNU Lesser General Public License
		* along with TAPLibrary. If not, see <http://www.gnu.org/licenses/>.
		*
		* Copyright 2012-2015 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
		* Copyright 2012-2016 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
		* Astronomisches Rechen Institut (ARI)
		*/

		import java.sql.ResultSet;
		import java.sql.Statement;

		import adql.query.ADQLQuery;
		import tap.TAPFactory;
		import tap.data.DataReadException;
		import tap.data.TableIterator;
		import tap.metadata.TAPColumn;
		import tap.metadata.TAPMetadata;
		import tap.metadata.TAPTable;
		import adql.query.ADQLQuery;

		/**
		* <p>Connection to the "database" (whatever is the type or whether it is linked to a true DBMS connection).</p>
		@@ -42,7 +45,7 @@ import adql.query.ADQLQuery;
		* </p>
		*
		* @author Grégory Mantelet (CDS;ARI)
		* @version 2.1 (11/2015)
		* @version 2.1 (04/2016)
		*/
		public interface DBConnection {

		@@ -107,7 +110,7 @@ public interface DBConnection {
		*
		* <h3>TAP_SCHEMA CREATION</h3>
		* <p>
		* This function is MAY drop and then re-create the schema TAP_SCHEMA and all
		* This function MAY drop and then re-create the schema TAP_SCHEMA and all
		* its tables listed in the TAP standard (TAP_SCHEMA.schemas, .tables, .columns, .keys and .key_columns).
		* <i>All other tables inside TAP_SCHEMA SHOULD NOT be modified!</i>
		* </p>
		@@ -115,7 +118,7 @@ public interface DBConnection {
		* <p>
		* The schema and the tables MUST be created using either the <b>standard definition</b> or the
		* <b>definition provided in the {@link TAPMetadata} object</b> (if provided). Indeed, if your definition of these TAP tables
		* is different from the standard (the standard + new elements), you MUST provide your modifications in parameter
		* is different from the standard (i.e. the standard + new elements), you MUST provide your modifications in parameter
		* through the {@link TAPMetadata} object so that they can be applied and taken into account in TAP_SCHEMA.
		* </p>
		*
		@@ -220,9 +223,18 @@ public interface DBConnection {
		*
		* <p>The result of this query must be formatted as a table, and so must be iterable using a {@link TableIterator}.</p>
		*
		* <p><i>note: the interpretation of the ADQL query is up to the implementation. In most of the case, it is just needed
		* <p><i>note: the interpretation of the ADQL query is up to the implementation. In most of the cases, it is just needed
		* to translate this ADQL query into an SQL query (understandable by the chosen DBMS).</i></p>
		*
		* <p><b>IMPORTANT:</b>
		* A {@link DBConnection} implementation may open resources to perform the query and get the result,
		* but it may especially KEEP them OPENED in order to let the returned {@link TableIterator} iterates on
		* the result set. So that closing these resources, the function {@link #endQuery()} should be called
		* when the result is no longer needed. A good implementation of {@link TableIterator} SHOULD call this
		* function when {@link TableIterator#close()} is called. <b>So, do not forget to call {@link TableIterator#close()}
		* when you do not need any more the query result.</b>
		* </p>
		*
		* @param adqlQuery ADQL query to execute.
		*
		* @return The table result.
		@@ -230,6 +242,9 @@ public interface DBConnection {
		* @throws DBException If any errors occurs while executing the query.
		*
		* @since 2.0
		*
		* @see #endQuery()
		* @see TableIterator#close()
		*/
		public TableIterator executeQuery(final ADQLQuery adqlQuery) throws DBException;

		@@ -272,12 +287,14 @@ public interface DBConnection {
		* <p>Stop the execution of the current query.</p>
		*
		* <p>
		* If asked. a rollback of the current transaction can also be performed
		* after the cancellation (if successful) of the query.
		* If asked, a rollback of the current transaction can also be performed
		* before the function returns. This rollback operation is totally independent
		* from the cancellation. It means that the rollback is always performed whatever
		* is the cancellation result (or whatever the cancellation can be performed or not).
		* </p>
		*
		* <p>
		* This function should <b>never</b> return any kind of exception. This is particularly important
		* This function should <b>never</b> throw any kind of exception. This is particularly important
		* in the following cases:
		* </p>
		* <ul>
		@@ -286,7 +303,7 @@ public interface DBConnection {
		* <li>no query is currently running</li>
		* <li>a rollback is not possible or failed</li>
		* </ul>
		* <p>However, if an exception occurs it may be directly logged at least as a WARNING.</p>
		* <p>However, if an exception occurs it should be directly logged at least as a WARNING.</p>
		*
		* @param rollback <code>true</code> to cancel the statement AND rollback the current connection transaction,
		* <code>false</code> to just cancel the statement.
		@@ -295,4 +312,36 @@ public interface DBConnection {
		*/
		public void cancel(final boolean rollback);

		/**
		* <p>End the last query performed by this {@link DBConnection} and free some associated resources
		* opened just for this last query.</p>
		*
		* <p>
		* Originally, this function aims to be called when the result of {@link #executeQuery(ADQLQuery)}
		* is no longer needed, in order to clean/free what the {@link DBConnection} needed to keep this
		* result set open. In other words, if we take the example of a JDBC connection, this function will
		* close the {@link ResultSet}, the {@link Statement} and will end any transaction eventually opened
		* by the {@link DBConnection} (for instance if a fetchSize is set).
		* </p>
		*
		* <p>
		* However, this function could also be used after any other operation performed by the {@link DBConnection}.
		* You should just be aware that, depending of the implementation, if a transaction has been opened, this
		* function may end it, which means generally that a rollback is performed.
		* </p>
		*
		* <p>
		* Similarly, since it is supposed to end any query lastly performed, this function must also cancel
		* any processing. So, the function {@link #cancel(boolean)} should be called.
		* </p>
		*
		* <p>
		* Finally, like {@link #cancel(boolean)}, this function should <b>never</b> throw any kind of exception.
		* If internally an exception occurs, it should be directly logged at least as a WARNING.
		* </p>
		*
		* @since 2.1
		*/
		public void endQuery();

		}

src/tap/db/JDBCConnection.java

+106 −45

Original line number	Diff line number	Diff line
		@@ -169,6 +169,13 @@ import uws.service.log.UWSLog.LogLevel;
		* To enable it, a simple call to {@link #setFetchSize(int)} is enough, whatever is the given value.
		* </i></p>
		*
		* <p><i>Note 3:
		* Generally set a fetch size starts a transaction in the database. So, after the result of the fetched query
		* is not needed any more, do not forget to call {@link #endQuery()} in order to end the implicitly opened transaction.
		* However, generally closing the returned {@link TableIterator} is fully enough (see the sources of
		* {@link ResultSetTableIterator#close()} for more details).
		* </i></p>
		*
		* @author Grégory Mantelet (CDS;ARI)
		* @version 2.1 (04/2016)
		* @since 2.0
		@@ -516,7 +523,7 @@ public class JDBCConnection implements DBConnection {
		*
		* <p><i>Note 1:
		* A failure of a rollback is not considered as a not supported cancellation feature by the JDBC driver or the DBMS.
		* So if the cancellation succeeds but a rollback fails, a next call of this function will still try cancelling the given statement.
		* So if the cancellation succeeds but a rollback fails, a next call of this function will still try canceling the given statement.
		* In case of a rollback failure, only a WARNING is written in the log file ; no exception is thrown.
		* </i></p>
		*
		@@ -541,7 +548,6 @@ public class JDBCConnection implements DBConnection {
		*/
		@Override
		public final void cancel(final boolean rollback){
		if (supportsCancel && stmt != null){
		synchronized(cancelled){
		cancelled = cancel(stmt, rollback);
		// Log the success of the cancellation:
		@@ -549,7 +555,6 @@ public class JDBCConnection implements DBConnection {
		logger.logDB(LogLevel.INFO, this, "CANCEL", "Query execution successfully stopped!", null);
		}
		}
		}

		/**
		* <p>Cancel (and rollback when asked and if possible) the given statement.</p>
		@@ -604,17 +609,8 @@ public class JDBCConnection implements DBConnection {
		}
		// Whatever happens, rollback all executed operations (only if rollback=true and if in a transaction ; that's to say if AutoCommit = false):
		finally{
		if (rollback && supportsTransaction){
		try{
		if (!connection.getAutoCommit()){
		connection.rollback();
		connection.setAutoCommit(true);
		}
		}catch(SQLException se){
		if (logger != null)
		logger.logDB(LogLevel.ERROR, this, "CANCEL", "Query execution successfully stopped BUT the rollback fails!", se);
		}
		}
		if (rollback && supportsTransaction)
		rollback();
		}
		}

		@@ -653,6 +649,18 @@ public class JDBCConnection implements DBConnection {
		}
		}

		@Override
		public void endQuery(){
		// Cancel the last query processing, if still running:
		cancel(stmt, false); // note: this function is called instead of cancel(false) in order to avoid a log message about the cancellation operation result.
		// Close the statement, if still opened:
		closeStatement();
		// Rollback the transaction, if one has been opened:
		rollback(false);
		// End the transaction (i.e. go back to autocommit=true), if one has been opened:
		endTransaction(false);
		}

		/* ********************* */
		/* INTERROGATION METHODS */
		/* ********************* */
		@@ -709,13 +717,8 @@ public class JDBCConnection implements DBConnection {
		}catch(Exception ex){
		// Close the ResultSet, if one was open:
		close(result);
		// Ensure the query is not running anymore and rollback the transaction:
		cancel(true);
		// If still not canceled, log this fact as an error:
		if (!isCancelled() && logger != null)
		logger.logDB(LogLevel.ERROR, this, "EXECUTE", "Unexpected error while EXECUTING SQL query!", null);
		// Close the open statement, if one was open:
		closeStatement();
		// End properly the query:
		endQuery();
		// Propagate the exception with an appropriate error message:
		if (ex instanceof SQLException)
		throw new DBException("Unexpected error while executing a SQL query: " + ex.getMessage(), ex);
		@@ -731,12 +734,13 @@ public class JDBCConnection implements DBConnection {
		/**
		* <p>Create a {@link TableIterator} instance which lets reading the given result table.</p>
		*
		* <p><b>Important note:</b>
		* This function also set to NULL the statement of this {@link JDBCConnection} instance: {@link #stmt}.
		* However, the statement is not closed ; it is just given to a {@link ResultSetTableIterator} iterator
		* which will close it in the same time as the given {@link ResultSet}, when its function
		* {@link ResultSetTableIterator#close()} is called.
		* </p>
		* <p><i>Note:
		* The statement currently opened is not closed by this function. Actually, it is still associated with
		* this {@link JDBCConnection}. However, this latter is provided to the {@link TableIterator} returned by
		* this function. Thus, when the {@link TableIterator#close()} is called, the function {@link #endQuery()}
		* will be called. It will then close the {@link ResultSet}, the {@link Statement} and end any opened
		* transaction (with rollback). See {@link #endQuery()} for more details.
		* </i></p>
		*
		* @param rs Result of an SQL query.
		* @param resultingColumns Metadata corresponding to each columns of the result.
		@@ -745,14 +749,12 @@ public class JDBCConnection implements DBConnection {
		*
		* @throws DataReadException If the metadata (columns count and types) can not be fetched
		* or if any other error occurs.
		*
		* @see ResultSetTableIterator#ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String)
		*/
		protected TableIterator createTableIterator(final ResultSet rs, final DBColumn[] resultingColumns) throws DataReadException{
		// Dis-associate the current Statement from this JDBCConnection instance:
		Statement itStmt = stmt;
		stmt = null;
		// Return a TableIterator wrapping the given ResultSet:
		try{
		return new ResultSetTableIterator(itStmt, rs, translator, dbms, resultingColumns);
		return new ResultSetTableIterator(this, rs, resultingColumns, translator, dbms);
		}catch(Throwable t){
		throw (t instanceof DataReadException) ? (DataReadException)t : new DataReadException(t);
		}
		@@ -852,7 +854,7 @@ public class JDBCConnection implements DBConnection {
		logger.logDB(LogLevel.ERROR, this, "LOAD_TAP_SCHEMA", "Impossible to create a Statement!", se);
		throw new DBException("Can not create a Statement!", se);
		}finally{
		cancel(true);
		cancel(stmt, true); // note: this function is called instead of cancel(true) in order to avoid a log message about the cancellation operation result.
		closeStatement();
		}

		@@ -2423,6 +2425,31 @@ public class JDBCConnection implements DBConnection {
		}
		}

		/**
		* <p>Rollback the current transaction.
		* The success or the failure of the rollback operation is always logged (except if no logger is available).</p>
		*
		* <p>
		* {@link #startTransaction()} must have been called before. If it's not the case the connection
		* may throw a {@link SQLException} which will be transformed into a {@link DBException} here.
		* </p>
		*
		* <p>If transactions are not supported by this connection, nothing is done.</p>
		*
		* <p><b><i>Important note:</b>
		* If any error interrupts the ROLLBACK operation, transactions will considered afterwards as not supported by this connection.
		* So, subsequent call to this function (and any other transaction related function) will never do anything.
		* </i></p>
		*
		* @throws DBException If it is impossible to rollback a transaction though transactions are supported by this connection..
		* If these are not supported, this error can never be thrown.
		*
		* @see #rollback(boolean)
		*/
		protected final void rollback(){
		rollback(true);
		}

		/**
		* <p>Rollback the current transaction.</p>
		*
		@@ -2438,23 +2465,52 @@ public class JDBCConnection implements DBConnection {
		* So, subsequent call to this function (and any other transaction related function) will never do anything.
		* </i></p>
		*
		* @param log <code>true</code> to log the success/failure of the rollback operation,
		* <code>false</code> to be quiet whatever happens.
		*
		* @throws DBException If it is impossible to rollback a transaction though transactions are supported by this connection..
		* If these are not supported, this error can never be thrown.
		*
		* @since 2.1
		*/
		protected void rollback(){
		protected void rollback(final boolean log){
		try{
		if (supportsTransaction){
		if (supportsTransaction && !connection.getAutoCommit()){
		connection.rollback();
		if (logger != null)
		if (log && logger != null)
		logger.logDB(LogLevel.INFO, this, "ROLLBACK", "Transaction ROLLBACKED.", null);
		}
		}catch(SQLException se){
		supportsTransaction = false;
		if (logger != null)
		if (log && logger != null)
		logger.logDB(LogLevel.ERROR, this, "ROLLBACK", "Transaction ROLLBACK impossible!", se);
		}
		}

		/**
		* <p>End the current transaction.
		* The success or the failure of the transaction ending operation is always logged (except if no logger is available).</p>
		*
		* <p>
		* Basically, if transactions are supported by this connection, the flag AutoCommit is just turned on.
		* </p>
		*
		* <p>If transactions are not supported by this connection, nothing is done.</p>
		*
		* <p><b><i>Important note:</b>
		* If any error interrupts the END TRANSACTION operation, transactions will be afterwards considered as not supported by this connection.
		* So, subsequent call to this function (and any other transaction related function) will never do anything.
		* </i></p>
		*
		* @throws DBException If it is impossible to end a transaction though transactions are supported by this connection.
		* If these are not supported, this error can never be thrown.
		*
		* @see #endTransaction(boolean)
		*/
		protected final void endTransaction(){
		endTransaction(true);
		}

		/**
		* <p>End the current transaction.</p>
		*
		@@ -2469,19 +2525,24 @@ public class JDBCConnection implements DBConnection {
		* So, subsequent call to this function (and any other transaction related function) will never do anything.
		* </i></p>
		*
		* @param log <code>true</code> to log the success/failure of the transaction ending operation,
		* <code>false</code> to be quiet whatever happens.
		*
		* @throws DBException If it is impossible to end a transaction though transactions are supported by this connection.
		* If these are not supported, this error can never be thrown.
		*
		* @since 2.1
		*/
		protected void endTransaction(){
		protected void endTransaction(final boolean log){
		try{
		if (supportsTransaction){
		connection.setAutoCommit(true);
		if (logger != null)
		if (log && logger != null)
		logger.logDB(LogLevel.INFO, this, "END_TRANSACTION", "Transaction ENDED.", null);
		}
		}catch(SQLException se){
		supportsTransaction = false;
		if (logger != null)
		if (log && logger != null)
		logger.logDB(LogLevel.ERROR, this, "END_TRANSACTION", "Transaction ENDing impossible!", se);
		}
		}