From 815e3638826f352ba40ba5fed4d3e37ebfb68a8e Mon Sep 17 00:00:00 2001 From: hieplq Date: Tue, 6 Feb 2024 19:05:49 +0700 Subject: [PATCH] IDEMPIERE-6016:test case to simulate transaction timeouts and ensure no open connections remain afterwards (#2219) * IDEMPIERE-6016:allow transaction monitor's scan timeout to a short duration * IDEMPIERE-6016:test case to simulate transaction timeouts and ensure no open connections remain afterwards * IDEMPIERE-6016:trx timeout make leak connection by call Connection.about but not call Connection.close(Hengsin review) replace abort with rollback use shorter duration for timeout test 2 add Isolatead and same thread execution class annotation * IDEMPIERE-6016:trx timeout make leak connection by call Connection.about but not call Connection.close(Hengsin review) --------- Co-authored-by: hengsin --- .../src/org/compiere/util/Trx.java | 27 ++-- .../src/org/idempiere/test/base/DBTest.java | 131 +++++++++++++----- 2 files changed, 102 insertions(+), 56 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/util/Trx.java b/org.adempiere.base/src/org/compiere/util/Trx.java index f4402b26a2..8dd3302b41 100644 --- a/org.adempiere.base/src/org/compiere/util/Trx.java +++ b/org.adempiere.base/src/org/compiere/util/Trx.java @@ -465,34 +465,25 @@ public class Trx } /** - * Rollback and End Transaction, Close Connection and Throws an Exception.
+ * Rollback and close transaction.
* This is means to be called by the timeout monitor and developer usually shouldn't call this directly. * @return true if success */ public boolean rollbackAndCloseOnTimeout() { - s_cache.remove(getTrxName()); - - //local - if (m_connection == null) - return true; - - // Close Connection + boolean success = false; try { - // Closes any physical connection to the database - m_connection.abort(Runnable::run); - // return to pool manage (pool will validate and re-connect to database) - m_connection.close(); - m_connection = null; - m_active = false; - fireAfterCloseEvent(); + rollback(true); } catch (SQLException e) { log.log(Level.SEVERE, m_trxName, e); } - log.config(m_trxName); - return true; + finally + { + success = close(); + } + return success; } /** @@ -830,7 +821,7 @@ public class Trx } /** Transaction timeout monitor class */ - static class TrxMonitor implements Runnable + public static class TrxMonitor implements Runnable { public void run() diff --git a/org.idempiere.test/src/org/idempiere/test/base/DBTest.java b/org.idempiere.test/src/org/idempiere/test/base/DBTest.java index da8e19b2f2..a1ce55c407 100644 --- a/org.idempiere.test/src/org/idempiere/test/base/DBTest.java +++ b/org.idempiere.test/src/org/idempiere/test/base/DBTest.java @@ -25,8 +25,14 @@ import java.sql.SQLException; import java.sql.Timestamp; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import org.adempiere.exceptions.DBException; +import org.compiere.Adempiere; import org.compiere.model.MBPartner; import org.compiere.model.MColumn; import org.compiere.model.MOrder; @@ -44,12 +50,17 @@ import org.idempiere.test.DictionaryIDs; import org.idempiere.test.LoginDetails; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.Isolated; /** * Test {@link org.compiere.util.DB} class * @author Teo Sarca, www.arhipac.ro * @author hengsin */ +@Isolated +@Execution(ExecutionMode.SAME_THREAD) public class DBTest extends AbstractTestCase { private static final int TEST_RECORD_ID = 103; @@ -340,44 +351,7 @@ public class DBTest extends AbstractTestCase rollback(); } } - - @Test - public void testTrxTimeout() - { - try { - MBPartner bp = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, getTrxName()); - DB.getDatabase().forUpdate(bp, 0); - - Exception exception = null; - Trx trx2 = Trx.get(Trx.createTrxName(), true); - MBPartner bp2 = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, trx2.getTrxName()); - try { - Thread thread = new Thread (() -> { - try { - Thread.sleep(5 * 1000); - } catch (InterruptedException e) { - } - if (trx2.isActive()) trx2.rollbackAndCloseOnTimeout(); - }); - thread.start(); - DB.getDatabase().forUpdate(bp2, 10); - } catch (Exception e) { - exception = e; - } finally { - trx2.close(); - } - assertNotNull(exception, "Exception not happens as expected"); - assertTrue(exception instanceof DBException, "Exception not instanceof DBException"); - DBException dbe = (DBException) exception; - if (DB.isPostgreSQL()) - assertTrue("08006".equals(dbe.getSQLState()), "Trx2 not timeout as expected: " + dbe.getSQLException().getMessage()); - else if (DB.isOracle()) - assertTrue("08003".equals(dbe.getSQLState()), "Trx2 not timeout as expected: " + dbe.getSQLException().getMessage()); - } finally { - rollback(); - } - } - + @Test public void testPostgreSQLSyncColumn() { if (!DB.isPostgreSQL() || !DB.getDatabase().isNativeMode()) @@ -408,6 +382,87 @@ public class DBTest extends AbstractTestCase return super.newLoginDetails(testInfo); } } + + @Test + public void testTrxTimeout() + { + try { + MBPartner bp = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, getTrxName()); + DB.getDatabase().forUpdate(bp, 0); + + Exception exception = null; + Trx trx2 = Trx.get(Trx.createTrxName(), true); + MBPartner bp2 = new MBPartner(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id, trx2.getTrxName()); + try { + Thread thread = new Thread (() -> { + try { + Thread.sleep(5 * 1000); + } catch (InterruptedException e) { + } + if (trx2.isActive()) trx2.rollbackAndCloseOnTimeout(); + }); + thread.start(); + DB.getDatabase().forUpdate(bp2, 10); + } catch (Exception e) { + exception = e; + } finally { + trx2.close(); + } + assertNotNull(exception, "Exception not happens as expected"); + assertTrue(exception instanceof DBException, "Exception not instanceof DBException"); + } finally { + rollback(); + } + } + + public static Pattern REG_ACTIVE_CONNECT = Pattern.compile("# Busy Connections:\\s*(\\d+)\\s*,", Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE); + public static int getNumConnectPerStatus (String poolStatus, Pattern patternStatus) { + int numActiveConn = -1; + try { + + Matcher regexMatcher = patternStatus.matcher(poolStatus); + if (regexMatcher.find()) { + String activeConnectionStr = regexMatcher.group(1); + numActiveConn = Integer.parseInt(activeConnectionStr); + } + } catch (PatternSyntaxException ex) { + // Syntax error in the regular expression + } + return numActiveConn; + } + + /** + * test case to simulate transaction timeouts and ensure no open connections remain afterwards + */ + @Test + public void testTrxTimeout2() { + //initial delay to give connection pool time to release active connections + try { + Thread.sleep(3000); + } catch (InterruptedException e) { + } + + //create a short duration monitor for testing + Trx.TrxMonitor monitor = new Trx.TrxMonitor(); + ScheduledFuture future = Adempiere.getThreadPoolExecutor().scheduleWithFixedDelay(monitor, 0, 6, TimeUnit.SECONDS); + int beforeActiveConnection = getNumConnectPerStatus(DB.getDatabase().getStatus(), REG_ACTIVE_CONNECT); + + Trx trx2 = Trx.get(Trx.createTrxName(), true); + trx2.setTimeout(3);// timeout after 3s + + DB.getSQLValueEx(trx2.getTrxName(), "SELECT 1 FROM DUAL");// to make transaction start + + try { + Thread.sleep(8000);//Wait for the transaction monitor to complete its task + + int afterActiveConnection = getNumConnectPerStatus(DB.getDatabase().getStatus(), REG_ACTIVE_CONNECT); + assertEquals(beforeActiveConnection, afterActiveConnection); + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + future.cancel(true); + } + } }