Fix for #159 - Refactor - cleanup clode when closing query only transaction + change default option

This commit is contained in:
Rob Bygrave
2014-07-05 22:10:53 +12:00
parent 1ddc103582
commit cceb7c035c
11 changed files with 162 additions and 144 deletions
@@ -15,6 +15,11 @@ import com.avaje.ebeaninternal.server.persist.BatchControl;
*/
public interface SpiTransaction extends Transaction {
/**
* End the transaction when had query only use.
*/
public void endQueryOnly();
/**
* Return the string prefix with the transactin id and label used in logging.
*/
@@ -32,8 +32,6 @@ public abstract class BeanRequest {
protected boolean createdTransaction;
protected boolean readOnly;
public BeanRequest(SpiEbeanServer ebeanServer, SpiTransaction t) {
this.ebeanServer = ebeanServer;
this.serverName = ebeanServer.getName();
@@ -62,29 +60,19 @@ public abstract class BeanRequest {
if (transaction == null || !transaction.isActive()) {
// create an implicit transaction to execute this query
transaction = ebeanServer.createServerTransaction(false, -1);
// commented out for performance reasons...
// TODO: review performance of trans.setReadOnly(true)
//if (readOnlyTransaction) {
// readOnly = true;
// transaction.setReadOnly(true);
//}
createdTransaction = true;
}
}
}
/**
* Commit this transaction if it was created for this request.
*/
public void commitTransIfRequired() {
if (createdTransaction) {
if (readOnly) {
transaction.rollback();
} else {
transaction.commit();
}
}
}
/**
* Commit this transaction if it was created for this request.
*/
public void commitTransIfRequired() {
if (createdTransaction) {
transaction.commit();
}
}
/**
* Rollback the transaction if it was created for this request.
@@ -1198,18 +1198,12 @@ public final class DefaultServer implements SpiEbeanServer {
}
SpiOrmQueryRequest<T> request = createQueryRequest(desc, spiQuery, t);
try {
request.initTransIfRequired();
return (T) request.findId();
T bean = (T) request.findId();
} finally {
request.endTransIfRequired();
return bean;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1256,15 +1250,10 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
Set<T> set = (Set<T>) request.findSet();
return (Set<T>) request.findSet();
} finally {
request.endTransIfRequired();
return set;
} catch (RuntimeException ex) {
// String stackTrace = throwablePrinter.print(ex);
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1280,15 +1269,10 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
Map<?, T> map = (Map<?, T>) request.findMap();
return (Map<?, T>) request.findMap();
} finally {
request.endTransIfRequired();
return map;
} catch (RuntimeException ex) {
// String stackTrace = throwablePrinter.print(ex);
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1303,14 +1287,10 @@ public final class DefaultServer implements SpiEbeanServer {
SpiOrmQueryRequest<T> request = createQueryRequest(Type.ROWCOUNT, query, t);
try {
request.initTransIfRequired();
int rowCount = request.findRowCount();
return request.findRowCount();
} finally {
request.endTransIfRequired();
return rowCount;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1326,14 +1306,10 @@ public final class DefaultServer implements SpiEbeanServer {
SpiOrmQueryRequest<T> request = createQueryRequest(Type.ID_LIST, query, t);
try {
request.initTransIfRequired();
List<Object> list = request.findIds();
return request.findIds();
} finally {
request.endTransIfRequired();
return list;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1434,10 +1410,8 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
request.findVisit(visitor);
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
} finally {
// do nothing - findVisit garuntee's cleanup of the transaction if required
}
}
@@ -1448,10 +1422,9 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
return request.findIterate();
// request.endTransIfRequired();
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
request.endTransIfRequired();
throw ex;
}
}
@@ -1468,14 +1441,10 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
List<T> list = request.findList();
return request.findList();
} finally {
request.endTransIfRequired();
return list;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1518,14 +1487,10 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
List<SqlRow> list = request.findList();
return request.findList();
} finally {
request.endTransIfRequired();
return list;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1535,14 +1500,10 @@ public final class DefaultServer implements SpiEbeanServer {
try {
request.initTransIfRequired();
Set<SqlRow> set = request.findSet();
return request.findSet();
} finally {
request.endTransIfRequired();
return set;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -1551,14 +1512,10 @@ public final class DefaultServer implements SpiEbeanServer {
RelationalQueryRequest request = new RelationalQueryRequest(this, relationalQueryEngine, query, t);
try {
request.initTransIfRequired();
Map<?, SqlRow> map = request.findMap();
return request.findMap();
} finally {
request.endTransIfRequired();
return map;
} catch (RuntimeException ex) {
request.rollbackTransIfRequired();
throw ex;
}
}
@@ -193,14 +193,12 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
/**
* Will end a locally created transaction.
* <p>
* It ends the transaction by using a rollback() as the transaction is known
* to be readOnly.
* It ends the query only transaction.
* </p>
*/
public void endTransIfRequired() {
if (createdTransaction) {
// we can rollback as readOnly transaction
transaction.rollback();
transaction.endQueryOnly();
}
}
@@ -40,15 +40,6 @@ public final class RelationalQueryRequest {
this.trans = (SpiTransaction) t;
}
/**
* Rollback the transaction if it was created for this request.
*/
public void rollbackTransIfRequired() {
if (createdTransaction) {
trans.rollback();
}
}
/**
* Create a transaction if none currently exists.
*/
@@ -58,10 +49,6 @@ public final class RelationalQueryRequest {
if (trans == null || !trans.isActive()) {
// create a local readOnly transaction
trans = ebeanServer.createServerTransaction(false, -1);
// commented out for performance reasons...
// TODO: review performance of trans.setReadOnly(true)
// trans.setReadOnly(true);
createdTransaction = true;
}
}
@@ -72,8 +59,7 @@ public final class RelationalQueryRequest {
*/
public void endTransIfRequired() {
if (createdTransaction) {
// we can rollback as a readOnly transaction.
trans.rollback();
trans.endQueryOnly();
}
}
@@ -45,8 +45,6 @@ public interface SpiOrmQueryRequest<T> {
*/
public void endTransIfRequired();
public void rollbackTransIfRequired();
/**
* Execute the query as findById.
*/
@@ -136,7 +136,7 @@ public class JdbcTransaction implements SpiTransaction {
try {
this.active = true;
this.id = id;
this.logPrefix = deriveLogPrefix(id,null);
this.logPrefix = deriveLogPrefix(id);
this.explicit = explicit;
this.manager = manager;
this.connection = connection;
@@ -152,23 +152,17 @@ public class JdbcTransaction implements SpiTransaction {
}
}
private static String deriveLogPrefix(String id, String label) {
private static String deriveLogPrefix(String id) {
StringBuilder sb = new StringBuilder();
sb.append("txn[");
if (id != null) {
sb.append(id);
}
sb.append("] ");
if (label != null) {
sb.append("label[").append(label).append("] ");
}
return sb.toString();
}
public void setLabel(String label) {
this.logPrefix = deriveLogPrefix(id,label);
}
public String getLogPrefix() {
return logPrefix;
}
@@ -541,16 +535,21 @@ public class JdbcTransaction implements SpiTransaction {
* Notify the transaction manager.
*/
protected void notifyCommit() {
if (manager == null) {
return;
}
if (queryOnly) {
manager.notifyOfQueryOnly(true, this, null);
} else {
manager.notifyOfCommit(this);
if (manager != null) {
if (queryOnly) {
manager.notifyOfQueryOnly(true, this, null);
} else {
manager.notifyOfCommit(this);
}
}
}
protected void notifyQueryOnly() {
if (manager != null) {
manager.notifyOfQueryOnly(true, this, null);
}
}
/**
* Rollback, Commit or Close for query only transaction.
* <p>
@@ -558,7 +557,7 @@ public class JdbcTransaction implements SpiTransaction {
* rollback or just close the connection for performance.
* </p>
*/
private void commitQueryOnly() {
private void connectionEndForQueryOnly() {
try {
switch (onQueryOnly) {
case ROLLBACK:
@@ -580,6 +579,22 @@ public class JdbcTransaction implements SpiTransaction {
}
}
/**
* End the transaction on a query only request.
*/
public void endQueryOnly() {
if (!isActive()) {
throw new IllegalStateException(illegalStateMessage);
}
try {
connectionEndForQueryOnly();
} finally {
// these will not throw an exception
deactivate();
notifyQueryOnly();
}
}
/**
* Commit the transaction.
*/
@@ -590,7 +605,7 @@ public class JdbcTransaction implements SpiTransaction {
try {
if (queryOnly) {
// can rollback or just close for performance
commitQueryOnly();
connectionEndForQueryOnly();
} else {
// commit
if (batchControl != null && !batchControl.isEmpty()) {
@@ -91,8 +91,6 @@ public class TransactionManager {
private final ClusterManager clusterManager;
//private final int commitDebugLevel;
private final String serverName;
/**
@@ -100,11 +98,11 @@ public class TransactionManager {
*/
private AtomicLong transactionCounter = new AtomicLong(1000);
private int clusterDebugLevel;
private final BulkEventListenerMap bulkEventListenerMap;
private int clusterDebugLevel;
private TransactionEventListener[] transactionEventListeners;
private final BulkEventListenerMap bulkEventListenerMap;
private TransactionEventListener[] transactionEventListeners;
/**
* Create the TransactionManager
@@ -130,7 +128,7 @@ public class TransactionManager {
this.prefix = GlobalProperties.get("transaction.prefix", "");
this.externalTransPrefix = GlobalProperties.get("transaction.prefix", "e");
String value = GlobalProperties.get("transaction.onqueryonly", "ROLLBACK").toUpperCase().trim();
String value = GlobalProperties.get("transaction.onqueryonly", "CLOSE").toUpperCase().trim();
this.onQueryOnly = getOnQueryOnly(value, dataSource);
initialiseHeartbeat();
@@ -171,7 +169,6 @@ public class TransactionManager {
*/
private OnQueryOnly getOnQueryOnly(String onQueryOnly, DataSource ds) {
if (onQueryOnly.equals("COMMIT")){
return OnQueryOnly.COMMIT;
}
@@ -429,9 +426,6 @@ public class TransactionManager {
logger.error(m, ex);
}
}
/**
* Process a Transaction that comes from another framework or local code.