Commit d9041712 authored by gmantele's avatar gmantele
Browse files

[TAP & UWS] 2 MAJOR BUGS FIXED (these bugs were affecting performances). 

1) [TAP & UWS] ]MAJOR BUG FIX: The abortion of an SQL query is now correctly
implemented. Before this fix, 2 mistakes prevented this clean abortion:
  a/ The thread was not cancelled because the SQL query execution was
blocking the thread. Then the thread could not treat the interruption though
it was flagged as interrupted.
  b/ The function UWSJob.isStopped() considered the job as stopped because
the interrupted flag was set, even though the thread was still processing
(and the database too). Because of that it returned true and the job phase
was ABORTED though the thread was still running.
  NOW:
  a/ TAPJob calls the function Statement.cancel() (if supported) in order
to cancel the SQL query execution properly inside the database.
  b/ The function UWSJob.isStopped() does not test any more the interrupted flag
and returns true only if the thread is really stopped.
  IN BRIEF: It is now sure that a job in the phase ABORTED is really stopped
(that's to say: thread stopped AND DB query execution stopped).

2) [TAP] BUG FIX: When the writing of a result is abnormaly interrupted for any
reason, the file which was being written is deleted.
parent 2b505506

src/tap/ADQLExecutor.java

+44 −5
Original line number Diff line number Diff line
@@ -104,7 +104,7 @@ import adql.query.ADQLQuery; 
 * </p>

 * 

 * @author Gr&eacute;gory Mantelet (CDS;ARI)

 * @version 2.0 (04/2015)

 * @version 2.1 (11/2015)

 */

public class ADQLExecutor {
@@ -224,9 +224,20 @@ public class ADQLExecutor { 
		try{

			return start();

		}catch(IOException ioe){

			if (thread.isInterrupted())

				return report;

			else

				throw new UWSException(ioe);

		}catch(TAPException te){

			if (thread.isInterrupted())

				return report;

			else

				throw new UWSException(te.getHttpErrorCode(), te);

		}catch(UWSException ue){

			if (thread.isInterrupted())

				return report;

			else

				throw ue;

		}

	}
@@ -249,6 +260,17 @@ public class ADQLExecutor { 
			dbConn = service.getFactory().getConnection(jobID);

	}



	/** 

	 * Cancel the current SQL query execution or result set fetching if any is currently running.

	 * If no such process is on going, this function has no effect.

	 * 

	 * @since 2.1

	 */

	public final void cancelQuery(){

		if (dbConn != null && progression == ExecutionProgression.EXECUTING_ADQL)

			dbConn.cancel(true);

	}



	/**

	 * <p>Start the synchronous processing of the ADQL query.</p>

	 *
@@ -625,12 +647,14 @@ public class ADQLExecutor { 
		}

		// CASE ASYNCHRONOUS:

		else{

			boolean completed = false;

			long start = -1, end = -1;

			Result result = null;

			JobThread jobThread = (JobThread)thread;

			try{

				// Create a UWS Result object to store the result

				// (the result will be stored in a file and this object is the association between the job and the result file):

				JobThread jobThread = (JobThread)thread;

				Result result = jobThread.createResult();

				result = jobThread.createResult();



				// Set the MIME type of the result format in the result description:

				result.setMimeType(formatter.getMimeType());
@@ -646,10 +670,25 @@ public class ADQLExecutor { 
				// Add the result description and link in the job description:

				jobThread.publishResult(result);



				completed = true;



				logger.logTAP(LogLevel.INFO, report, "RESULT_WRITTEN", "Result formatted (in " + formatter.getMimeType() + " ; " + (report.nbRows < 0 ? "?" : report.nbRows) + " rows ; " + ((report.resultingColumns == null) ? "?" : report.resultingColumns.length) + " columns) in " + ((start <= 0 || end <= 0) ? "?" : (end - start)) + "ms!", null);



			}catch(IOException ioe){

				// Propagate the exception:

				throw new UWSException(UWSException.INTERNAL_SERVER_ERROR, ioe, "Impossible to write in the file into the result of the job " + report.jobID + " must be written!");

			}finally{

				if (!completed){

					// Delete the result file (it is either incomplete or incorrect ;

					// it is then not reliable and is anyway not associated with the job and so could not be later deleted when the job will be):

					if (result != null){

						try{

							service.getFileManager().deleteResult(result, jobThread.getJob());

						}catch(IOException ioe){

							logger.logTAP(LogLevel.ERROR, report, "WRITING_RESULT", "The result writting has failed and the produced partial result must be deleted, but this deletion also failed! (job: " + report.jobID + ")", ioe);

						}

					}

				}

			}

		}

	}

src/tap/TAPJob.java

+26 −1
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ import uws.service.log.UWSLog.LogLevel; 
 * </p>

 * 

 * @author Gr&eacute;gory Mantelet (CDS;ARI)

 * @version 2.0 (04/2015)

 * @version 2.1 (11/2015)

 */

public class TAPJob extends UWSJob {

	private static final long serialVersionUID = 1L;
@@ -341,6 +341,31 @@ public class TAPJob extends UWSJob { 
		}

	}



	/** @since 2.1 */

	@Override

	protected void stop(){

		if (!isStopped()){

			synchronized(thread){

				stopping = true;



				// Interrupts the thread:

				thread.interrupt();



				// Cancel the query execution if any currently running:

				((AsyncThread)thread).executor.cancelQuery();



				// Wait a little for its end:

				if (waitForStop > 0){

					try{

						thread.join(waitForStop);

					}catch(InterruptedException ie){

						getLogger().logJob(LogLevel.WARNING, this, "END", "Unexpected InterruptedException while waiting for the end of the execution of the job \"" + jobId + "\" (thread ID: " + thread.getId() + ")!", ie);

					}

				}

			}

		}

	}



	/**

	 * This exception is thrown by a job execution when no database connection are available anymore.

	 *

src/tap/config/ConfigurableTAPFactory.java

+4 −1
Original line number Diff line number Diff line
@@ -74,7 +74,7 @@ import adql.translator.PostgreSQLTranslator; 
 * </p>

 * 

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 2.0 (04/2015)

 * @version 2.1 (11/2015)

 * @since 2.0

 */

public final class ConfigurableTAPFactory extends AbstractTAPFactory {
@@ -281,6 +281,9 @@ public final class ConfigurableTAPFactory extends AbstractTAPFactory { 
	@Override

	public void freeConnection(DBConnection conn){

		try{

			// Cancel any possible query that could be running:

			conn.cancel(false);

			// 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){

			service.getLogger().error("Can not close properly the connection \"" + conn.getID() + "\"!", se);

src/tap/data/ResultSetTableIterator.java

+231 −10
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ package tap.data; 
import java.sql.ResultSet;

import java.sql.ResultSetMetaData;

import java.sql.SQLException;

import java.sql.Statement;

import java.sql.Timestamp;

import java.util.NoSuchElementException;
@@ -42,11 +43,16 @@ import adql.translator.JDBCTranslator; 
 * </i></p>

 * 

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 2.1 (10/2015)

 * @version 2.1 (11/2015)

 * @since 2.0

 */

public class ResultSetTableIterator implements TableIterator {



	/** Statement associated with the ResultSet/Dataset to read.

	 * <i>MAY be NULL</i>

	 * @since 2.1 */

	private final Statement stmt;



	/** ResultSet/Dataset to read. */

	private final ResultSet data;
@@ -90,10 +96,41 @@ public class ResultSetTableIterator implements TableIterator { 
	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(ResultSet, JDBCTranslator, String, DBColumn[])

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 */

	public ResultSetTableIterator(final ResultSet dataSet) throws NullPointerException, DataReadException{

		this(dataSet, null, null, null);

		this(null, dataSet, null, null, null);

	}



	/**

	 * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p>

	 * 

	 * <p>

	 * 	In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype

	 * 	from the DBMS column datatype (using {@link #convertType(int, String, String)}).

	 * </p>

	 * 

	 * <h3>Type guessing</h3>

	 * 

	 * <p>

	 * 	In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)}

	 * 	which deals with the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby.

	 * 	This conversion is therefore not as precise as the one expected by a translator. That's why it is recommended

	 * 	to use one of the constructor having a {@link JDBCTranslator} in parameter.

	 * </p>

	 * 

	 * @param dataSet		Dataset over which this iterator must iterate.

	 * 

	 * @throws NullPointerException	If NULL is given in parameter.

	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 * 

	 * @since 2.1

	 */

	public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet) throws NullPointerException, DataReadException{

		this(stmt, dataSet, null, null, null);

	}



	/**
@@ -127,10 +164,49 @@ public class ResultSetTableIterator implements TableIterator { 
	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see ResultSetTableIterator#ResultSetTableIterator(ResultSet, JDBCTranslator, String, DBColumn[])

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 */

	public ResultSetTableIterator(final ResultSet dataSet, final String dbms) throws NullPointerException, DataReadException{

		this(dataSet, null, dbms, null);

		this(null, dataSet, null, dbms, null);

	}



	/**

	 * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p>

	 * 

	 * <p>

	 * 	In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype

	 * 	from the DBMS column datatype (using {@link #convertType(int, String, String)}).

	 * </p>

	 * 

	 * <h3>Type guessing</h3>

	 * 

	 * <p>

	 * 	In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)}

	 * 	which deals with the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby.

	 * 	This conversion is therefore not as precise as the one expected by a translator. That's why it is recommended

	 * 	to use one of the constructor having a {@link JDBCTranslator} in parameter.

	 * </p>

	 * 

	 * <p><i><b>Important</b>:

	 * 	The second parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}.

	 * 	<b>This parameter is really used ONLY when the DBMS is SQLite ("sqlite").</b>

	 * 	Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the

	 * 	ResultSet is coming. Without this information, type guessing will be unpredictable! 

	 * </i></p>

	 * 

	 * @param dataSet		Dataset over which this iterator must iterate.

	 * @param dbms			Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i>

	 * 

	 * @throws NullPointerException	If NULL is given in parameter.

	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 * 

	 * @since 2.1

	 */

	public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final String dbms) throws NullPointerException, DataReadException{

		this(stmt, dataSet, null, dbms, null);

	}



	/**
@@ -159,10 +235,44 @@ public class ResultSetTableIterator implements TableIterator { 
	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see ResultSetTableIterator#ResultSetTableIterator(ResultSet, JDBCTranslator, String, DBColumn[])

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 */

	public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator) throws NullPointerException, DataReadException{

		this(dataSet, translator, null, null);

		this(null, dataSet, translator, null, null);

	}



	/**

	 * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p>

	 * 

	 * <p>

	 * 	In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype

	 * 	from the DBMS column datatype (using {@link #convertType(int, String, String)}).

	 * </p>

	 * 

	 * <h3>Type guessing</h3>

	 * 

	 * <p>

	 * 	In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)}

	 * 	which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])})

	 * 	if not NULL. However if no translator is provided, this function will proceed to a default conversion

	 * 	using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby.

	 * 	This conversion is therefore not as precise as the one expected by the translator.

	 * </p>

	 * 

	 * @param dataSet		Dataset over which this iterator must iterate.

	 * @param translator	The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert

	 *                  	JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> 

	 * 

	 * @throws NullPointerException	If NULL is given in parameter.

	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 * 

	 * @since 2.1

	 */

	public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator) throws NullPointerException, DataReadException{

		this(stmt, dataSet, translator, null, null);

	}



	/**
@@ -199,10 +309,52 @@ public class ResultSetTableIterator implements TableIterator { 
	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see ResultSetTableIterator#ResultSetTableIterator(ResultSet, JDBCTranslator, String, DBColumn[])

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 */

	public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{

		this(dataSet, translator, dbms, null);

		this(null, dataSet, translator, dbms, null);

	}



	/**

	 * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p>

	 * 

	 * <p>

	 * 	In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype

	 * 	from the DBMS column datatype (using {@link #convertType(int, String, String)}).

	 * </p>

	 * 

	 * <h3>Type guessing</h3>

	 * 

	 * <p>

	 * 	In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)}

	 * 	which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])})

	 * 	if not NULL. However if no translator is provided, this function will proceed to a default conversion

	 * 	using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby.

	 * 	This conversion is therefore not as precise as the one expected by the translator.

	 * </p>

	 * 

	 * <p><i><b>Important</b>:

	 * 	The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}.

	 * 	<b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b>

	 * 	Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the

	 * 	ResultSet is coming. Without this information, type guessing will be unpredictable! 

	 * </i></p>

	 * 

	 * @param dataSet		Dataset over which this iterator must iterate.

	 * @param translator	The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert

	 *                  	JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> 

	 * @param dbms			Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i>

	 * 

	 * @throws NullPointerException	If NULL is given in parameter.

	 * @throws DataReadException	If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 * 

	 * @since 2.1

	 */

	public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{

		this(stmt, dataSet, translator, dbms, null);

	}



	/**
@@ -256,12 +408,74 @@ public class ResultSetTableIterator implements TableIterator { 
	 * @throws DataReadException	If the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[])

	 */

	public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{

		this(null, dataSet, translator, dbms, resultMeta);

	}



	/**

	 * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p>

	 * 

	 * <p>

	 * 	In order to provide the metadata through {@link #getMetadata()}, this constructor is reading first the given metadata (if any),

	 * 	and then, try to guess the datatype from the DBMS column datatype (using {@link #convertType(int, String, String)}).

	 * </p>

	 * 

	 * <h3>Provided metadata</h3>

	 * 

	 * <p>The third parameter of this constructor aims to provide the metadata expected for each column of the ResultSet.</p>

	 * 

	 * <p>

	 * 	For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn}

	 * 	instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept

	 * 	but the type will be guessed from the type provide by the ResultSetMetadata.

	 * </p>

	 * 

	 * <p><i>Note:

	 * 	If this parameter is incomplete (array length less than the column count returned by the ResultSet or some array items are NULL),

	 * 	column metadata will be associated in the same order as the ResultSet columns. Missing metadata will be built from the

	 * 	{@link ResultSetMetaData} and so the types will be guessed.

	 * </i></p>

	 * 

	 * <h3>Type guessing</h3>

	 * 

	 * <p>

	 * 	In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)}

	 * 	which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])})

	 * 	if not NULL. However if no translator is provided, this function will proceed to a default conversion

	 * 	using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby.

	 * 	This conversion is therefore not as precise as the one expected by the translator.

	 * </p>

	 * 

	 * <p><i><b>Important</b>:

	 * 	The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}.

	 * 	<b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b>

	 * 	Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the

	 * 	ResultSet is coming. Without this information, type guessing will be unpredictable! 

	 * </i></p>

	 * 

	 * @param dataSet		Dataset over which this iterator must iterate.

	 * @param translator	The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert

	 *                  	JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> 

	 * @param dbms			Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i>

	 * @param resultMeta	List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i>

	 * 

	 * @throws NullPointerException	If NULL is given in parameter.

	 * @throws DataReadException	If the metadata (columns count and types) can not be fetched.

	 * 

	 * @see #convertType(int, String, String)

	 * 

	 * @since 2.1

	 */

	public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator, final String dbms, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{

		// A dataset MUST BE provided:

		if (dataSet == null)

			throw new NullPointerException("Missing ResultSet object over which to iterate!");



		// Set the associated statement:

		this.stmt = stmt;



		// Keep a reference to the ResultSet:

		data = dataSet;
@@ -296,10 +510,17 @@ public class ResultSetTableIterator implements TableIterator { 


	@Override

	public void close() throws DataReadException{

		boolean rsClosed = false;

		try{

			data.close();

			rsClosed = true;

			if (stmt != null)

				stmt.close();

		}catch(SQLException se){

			if (!rsClosed)

				throw new DataReadException("Can not close the iterated ResultSet!", se);

			else

				throw new DataReadException("ResultSet successfully closed but impossible to closed the associated Statement!", se);

		}

	}

src/tap/db/DBConnection.java

+28 −1
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ import adql.query.ADQLQuery; 
 * </p>

 * 

 * @author Gr&eacute;gory Mantelet (CDS;ARI)

 * @version 2.0 (03/2015)

 * @version 2.1 (11/2015)

 */

public interface DBConnection {
@@ -268,4 +268,31 @@ public interface DBConnection { 
	 */

	public void setFetchSize(final int size);



	/**

	 * <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.

	 * </p>

	 * 

	 * <p>

	 * 	This function should <b>never</b> return any kind of exception. This is particularly important

	 * 	in the following cases:

	 * </p>

	 * <ul>

	 * 	<li>this function is not implemented</li>

	 * 	<li>the database driver or another API used to interact with a "database" does not support the cancel operation</li>

	 * 	<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>

	 * 

	 * @param rollback	<code>true</code> to cancel the statement AND rollback the current connection transaction,

	 *                	<code>false</code> to just cancel the statement.

	 * 

	 * @since 2.1

	 */

	public void cancel(final boolean rollback);



}