From 851f1c98b020223e4e8e27668378376f52aff502 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 31 Jan 2018 14:02:50 +1300 Subject: [PATCH] #1245 - Refactor internals - move Transaction ProfileStream creation Stops the profileId parameter pollution, enables to potentially use transaction label and runtime toggling of collecting transaction profiling detail. --- .../io/ebeaninternal/api/SpiTransaction.java | 5 ++++ .../api/SpiTransactionProxy.java | 5 ++++ .../server/core/DefaultServer.java | 8 +++--- .../AutoCommitTransactionManager.java | 2 +- .../DocStoreTransactionManager.java | 6 ++--- .../ExplicitTransactionManager.java | 2 +- .../ImplicitReadOnlyTransaction.java | 5 ++++ .../server/transaction/JdbcTransaction.java | 17 +++++------- .../server/transaction/NoTransaction.java | 5 ++++ .../transaction/TransactionFactory.java | 2 +- .../transaction/TransactionFactoryBasic.java | 13 ++++------ .../TransactionFactoryBasicWithRead.java | 1 - .../transaction/TransactionFactoryTenant.java | 10 +++---- .../transaction/TransactionManager.java | 26 ++++++++++++------- 14 files changed, 62 insertions(+), 45 deletions(-) diff --git a/src/main/java/io/ebeaninternal/api/SpiTransaction.java b/src/main/java/io/ebeaninternal/api/SpiTransaction.java index a8988662f..715b9387f 100644 --- a/src/main/java/io/ebeaninternal/api/SpiTransaction.java +++ b/src/main/java/io/ebeaninternal/api/SpiTransaction.java @@ -300,6 +300,11 @@ public interface SpiTransaction extends Transaction { */ void profileEvent(SpiProfileTransactionEvent event); + /** + * Set the profileStream to catch and time all the events for this transaction. + */ + void setProfileStream(ProfileStream profileStream); + /** * Return the stream that profiling events are written to. */ diff --git a/src/main/java/io/ebeaninternal/api/SpiTransactionProxy.java b/src/main/java/io/ebeaninternal/api/SpiTransactionProxy.java index 1743a1b7b..b9907aa3a 100644 --- a/src/main/java/io/ebeaninternal/api/SpiTransactionProxy.java +++ b/src/main/java/io/ebeaninternal/api/SpiTransactionProxy.java @@ -60,6 +60,11 @@ abstract class SpiTransactionProxy implements SpiTransaction { transaction.profileEvent(event); } + @Override + public void setProfileStream(ProfileStream profileStream) { + transaction.setProfileStream(profileStream); + } + @Override public ProfileStream profileStream() { return transaction.profileStream(); diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 018d63ac2..2c3c21d48 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -652,8 +652,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { */ @Override public Transaction createTransaction() { - - return transactionManager.createTransaction(0, true, -1); + return transactionManager.createTransaction(true, -1); } /** @@ -664,8 +663,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { */ @Override public Transaction createTransaction(TxIsolation isolation) { - - return transactionManager.createTransaction(0, true, isolation.getLevel()); + return transactionManager.createTransaction(true, isolation.getLevel()); } @Override @@ -758,7 +756,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { @Override public Transaction beginTransaction(TxIsolation isolation) { // start an explicit transaction - SpiTransaction t = transactionManager.createTransaction(0, true, isolation.getLevel()); + SpiTransaction t = transactionManager.createTransaction(true, isolation.getLevel()); try { // note that we are not supporting nested scoped transactions in this case transactionManager.set(t); diff --git a/src/main/java/io/ebeaninternal/server/transaction/AutoCommitTransactionManager.java b/src/main/java/io/ebeaninternal/server/transaction/AutoCommitTransactionManager.java index 6625dd997..ca862ebfa 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/AutoCommitTransactionManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/AutoCommitTransactionManager.java @@ -19,7 +19,7 @@ public class AutoCommitTransactionManager extends TransactionManager { * Create an autoCommit based Transaction. */ @Override - protected SpiTransaction createTransaction(int profileId, boolean explicit, Connection c, long id) { + protected SpiTransaction createTransaction(boolean explicit, Connection c, long id) { return new AutoCommitJdbcTransaction(prefix + id, explicit, c, this); } diff --git a/src/main/java/io/ebeaninternal/server/transaction/DocStoreTransactionManager.java b/src/main/java/io/ebeaninternal/server/transaction/DocStoreTransactionManager.java index 8beb2b3bb..53f1f5c04 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/DocStoreTransactionManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/DocStoreTransactionManager.java @@ -22,9 +22,9 @@ public class DocStoreTransactionManager extends TransactionManager { } @Override - public SpiTransaction createTransaction(int profileId, boolean explicit, int isolationLevel) { + public SpiTransaction createTransaction(boolean explicit, int isolationLevel) { long id = counter.incrementAndGet(); - return createTransaction(profileId, explicit, null, id); + return createTransaction(explicit, null, id); } @Override @@ -33,7 +33,7 @@ public class DocStoreTransactionManager extends TransactionManager { } @Override - protected SpiTransaction createTransaction(int profileId, boolean explicit, Connection c, long id) { + protected SpiTransaction createTransaction(boolean explicit, Connection c, long id) { return new DocStoreOnlyTransaction(prefix + id, explicit, this); } } diff --git a/src/main/java/io/ebeaninternal/server/transaction/ExplicitTransactionManager.java b/src/main/java/io/ebeaninternal/server/transaction/ExplicitTransactionManager.java index df710279b..8d082c3e3 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/ExplicitTransactionManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/ExplicitTransactionManager.java @@ -18,7 +18,7 @@ public class ExplicitTransactionManager extends TransactionManager { * Create a ExplicitJdbcTransaction. */ @Override - protected SpiTransaction createTransaction(int profileId, boolean explicit, Connection c, long id) { + protected SpiTransaction createTransaction(boolean explicit, Connection c, long id) { return new ExplicitJdbcTransaction(prefix + id, explicit, c, this); } diff --git a/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java b/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java index 526437a7a..0e95880cb 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java +++ b/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java @@ -104,6 +104,11 @@ class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEventCode // do nothing } + @Override + public void setProfileStream(ProfileStream profileStream) { + // do nothing + } + @Override public ProfileStream profileStream() { return null; diff --git a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java index 75055fb48..322f70725 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java @@ -177,25 +177,17 @@ public class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { protected DocStoreTransaction docStoreTxn; - private final ProfileStream profileStream; + private ProfileStream profileStream; protected ProfileLocation profileLocation; protected final long startNanos; - /** - * Create without ProfileStream option (no profiling). - */ - public JdbcTransaction(String id, boolean explicit, Connection connection, TransactionManager manager) { - this(null, id, explicit, connection, manager); - } - /** * Create a new JdbcTransaction. */ - public JdbcTransaction(ProfileStream profileStream, String id, boolean explicit, Connection connection, TransactionManager manager) { + public JdbcTransaction(String id, boolean explicit, Connection connection, TransactionManager manager) { try { - this.profileStream = profileStream; this.active = true; this.id = id; this.logPrefix = deriveLogPrefix(id); @@ -246,6 +238,11 @@ public class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { } } + @Override + public void setProfileStream(ProfileStream profileStream) { + this.profileStream = profileStream; + } + @Override public ProfileStream profileStream() { return profileStream; diff --git a/src/main/java/io/ebeaninternal/server/transaction/NoTransaction.java b/src/main/java/io/ebeaninternal/server/transaction/NoTransaction.java index 5a2e202de..9ee156066 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/NoTransaction.java +++ b/src/main/java/io/ebeaninternal/server/transaction/NoTransaction.java @@ -425,6 +425,11 @@ class NoTransaction implements SpiTransaction { } + @Override + public void setProfileStream(ProfileStream profileStream) { + + } + @Override public ProfileStream profileStream() { return null; diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactory.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactory.java index cbcddc681..a1593736d 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactory.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactory.java @@ -34,7 +34,7 @@ abstract class TransactionFactory { /** * Return a new transaction. */ - abstract SpiTransaction createTransaction(int profileId, boolean explicit, int isolationLevel); + abstract SpiTransaction createTransaction(boolean explicit, int isolationLevel); /** * Set the Transaction Isolation level if required. diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasic.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasic.java index d93c07753..3b552bf27 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasic.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasic.java @@ -13,13 +13,10 @@ import java.sql.SQLException; */ class TransactionFactoryBasic extends TransactionFactory { - final DataSourceSupplier dataSourceSupplier; - private final DataSource dataSource; TransactionFactoryBasic(TransactionManager manager, DataSourceSupplier dataSourceSupplier) { super(manager); - this.dataSourceSupplier = dataSourceSupplier; this.dataSource = dataSourceSupplier.getDataSource(); } @@ -29,7 +26,7 @@ class TransactionFactoryBasic extends TransactionFactory { Connection connection = null; try { connection = dataSource.getConnection(); - return create(0, false, connection); + return create(false, connection); } catch (PersistenceException ex) { JdbcClose.close(connection); @@ -40,11 +37,11 @@ class TransactionFactoryBasic extends TransactionFactory { } @Override - public SpiTransaction createTransaction(int profileId, boolean explicit, int isolationLevel) { + public SpiTransaction createTransaction(boolean explicit, int isolationLevel) { Connection connection = null; try { connection = dataSource.getConnection(); - SpiTransaction t = create(profileId, explicit, connection); + SpiTransaction t = create(explicit, connection); return setIsolationLevel(t, explicit, isolationLevel); } catch (PersistenceException ex) { JdbcClose.close(connection); @@ -54,8 +51,8 @@ class TransactionFactoryBasic extends TransactionFactory { } } - private SpiTransaction create(int profileId, boolean explicit, Connection c) { - return manager.createTransaction(profileId, explicit, c, counter.incrementAndGet()); + private SpiTransaction create(boolean explicit, Connection c) { + return manager.createTransaction(explicit, c, counter.incrementAndGet()); } } diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasicWithRead.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasicWithRead.java index 57048bc28..19926ffea 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasicWithRead.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryBasicWithRead.java @@ -18,7 +18,6 @@ import java.sql.SQLException; */ class TransactionFactoryBasicWithRead extends TransactionFactoryBasic { - private final DataSource readOnlyDataSource; TransactionFactoryBasicWithRead(TransactionManager manager, DataSourceSupplier dataSourceSupplier) { diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryTenant.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryTenant.java index 68a6ed579..c1fa5f670 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryTenant.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryTenant.java @@ -25,17 +25,17 @@ class TransactionFactoryTenant extends TransactionFactory { @Override public SpiTransaction createQueryTransaction(Object tenantId) { - return create(0,false, tenantId); + return create(false, tenantId); } @Override - public SpiTransaction createTransaction(int profileId, boolean explicit, int isolationLevel) { + public SpiTransaction createTransaction(boolean explicit, int isolationLevel) { - SpiTransaction t = create(profileId, explicit, null); + SpiTransaction t = create(explicit, null); return setIsolationLevel(t, explicit, isolationLevel); } - private SpiTransaction create(int profileId, boolean explicit, Object tenantId) { + private SpiTransaction create(boolean explicit, Object tenantId) { Connection connection = null; try { if (tenantId == null) { @@ -43,7 +43,7 @@ class TransactionFactoryTenant extends TransactionFactory { tenantId = tenantProvider.currentId(); } connection = dataSourceSupplier.getConnection(tenantId); - SpiTransaction transaction = manager.createTransaction(profileId, explicit, connection, counter.incrementAndGet()); + SpiTransaction transaction = manager.createTransaction(explicit, connection, counter.incrementAndGet()); transaction.setTenantId(tenantId); return transaction; diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java index cc2608919..1ff2c453e 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java @@ -132,7 +132,6 @@ public class TransactionManager implements SpiTransactionManager { private final SpiProfileHandler profileHandler; - private final MetricFactory metricFactory; private final TimedMetric txnMain; private final TimedMetric txnReadOnly; private final TimedMetricMap txnNamed; @@ -169,7 +168,8 @@ public class TransactionManager implements SpiTransactionManager { CurrentTenantProvider tenantProvider = options.config.getCurrentTenantProvider(); this.transactionFactory = TransactionFactoryBuilder.build(this, dataSourceSupplier, tenantProvider); - this.metricFactory = MetricFactory.get(); + + MetricFactory metricFactory = MetricFactory.get(); this.txnMain = metricFactory.createTimedMetric("txn.main"); this.txnReadOnly = metricFactory.createTimedMetric("txn.readonly"); this.txnNamed = metricFactory.createTimedMetricMap("txn.named."); @@ -180,7 +180,7 @@ public class TransactionManager implements SpiTransactionManager { /** * Create a new scoped transaction. */ - public ScopedTransaction createScopedTransaction() { + private ScopedTransaction createScopedTransaction() { return new ScopedTransaction(scopeManager); } @@ -327,10 +327,13 @@ public class TransactionManager implements SpiTransactionManager { /** * Create a new Transaction. */ - public SpiTransaction createTransaction(int profileId, boolean explicit, int isolationLevel) { - return transactionFactory.createTransaction(profileId, explicit, isolationLevel); + public SpiTransaction createTransaction(boolean explicit, int isolationLevel) { + return transactionFactory.createTransaction(explicit, isolationLevel); } + /** + * Create a new Transaction for query only purposes (can use read only datasource). + */ public SpiTransaction createQueryTransaction(Object tenantId) { return transactionFactory.createQueryTransaction(tenantId); } @@ -338,10 +341,9 @@ public class TransactionManager implements SpiTransactionManager { /** * Create a new transaction. */ - protected SpiTransaction createTransaction(int profileId, boolean explicit, Connection c, long id) { + protected SpiTransaction createTransaction(boolean explicit, Connection c, long id) { - ProfileStream profileStream = profileHandler.createProfileStream(profileId); - return new JdbcTransaction(profileStream, prefix + id, explicit, c, this); + return new JdbcTransaction(prefix + id, explicit, c, this); } /** @@ -544,7 +546,7 @@ public class TransactionManager implements SpiTransactionManager { * Begin an implicit transaction. */ public SpiTransaction beginServerTransaction() { - SpiTransaction t = createTransaction(0, false, -1); + SpiTransaction t = createTransaction(false, -1); scopeManager.set(t); return t; } @@ -604,7 +606,7 @@ public class TransactionManager implements SpiTransactionManager { transaction = NoTransaction.INSTANCE; break; default: - transaction = createTransaction(txScope.getProfileId(), true, txScope.getIsolationLevel()); + transaction = createTransaction(true, txScope.getIsolationLevel()); initNewTransaction(transaction, txScope); } } @@ -625,6 +627,10 @@ public class TransactionManager implements SpiTransactionManager { if (label != null) { transaction.setLabel(label); } + int profileId = txScope.getProfileId(); + if (profileId > 0) { + transaction.setProfileStream(profileHandler.createProfileStream(profileId)); + } ProfileLocation profileLocation = txScope.getProfileLocation(); if (profileLocation != null) { profileLocation.obtain();