From 0c37fc6e5744a8d84d9d343ac29323844f682b6e Mon Sep 17 00:00:00 2001 From: Albert Lee Date: Fri, 18 Feb 2011 16:59:16 +0000 Subject: [PATCH] OPENJPA-1943 - Apply query timeout value to pessimistic row lock statement execution. git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@1072061 13f79535-47bb-0310-9956-ffa450edef68 --- .../jdbc/sql/sql-error-state-codes.xml | 2 +- .../org/apache/openjpa/kernel/BrokerImpl.java | 6 +- .../openjpa/kernel/DelegatingBroker.java | 8 ++ .../apache/openjpa/kernel/StoreContext.java | 9 ++ .../test/AbstractPersistenceTestCase.java | 6 + .../lockmgr/SequencedActionsTest.java | 110 +++++++++++++----- .../lockmgr/TestMixedLockManagerDeadlock.java | 24 ++-- .../lockmgr/TestPessimisticLocks.java | 52 ++++----- .../persistence/EntityManagerImpl.java | 6 +- .../apache/openjpa/persistence/QueryImpl.java | 79 +++++++++---- 10 files changed, 207 insertions(+), 95 deletions(-) diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml index 0c33ae756..1a27b0904 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml @@ -47,7 +47,7 @@ - 1204,1205,1222,HY008 + 1204,1205,1222,HY008,40001 544,2601,2627,8114,8115 23000 diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java index ba4de640f..c0144fa21 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java @@ -455,10 +455,14 @@ public class BrokerImpl } public FetchConfiguration pushFetchConfiguration() { + return pushFetchConfiguration(null); + } + + public FetchConfiguration pushFetchConfiguration(FetchConfiguration fc) { if (_fcs == null) _fcs = new LinkedList(); _fcs.add(_fc); - _fc = (FetchConfiguration) _fc.clone(); + _fc = (FetchConfiguration) (fc != null ? fc : _fc).clone(); return _fc; } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingBroker.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingBroker.java index 51eb0c823..d0ff9b501 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingBroker.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingBroker.java @@ -154,6 +154,14 @@ public class DelegatingBroker } } + public FetchConfiguration pushFetchConfiguration(FetchConfiguration fc) { + try { + return _broker.pushFetchConfiguration(fc); + } catch (RuntimeException re) { + throw translate(re); + } + } + public void popFetchConfiguration() { try { _broker.popFetchConfiguration(); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java index c2ed7b741..bb85627b0 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java @@ -73,6 +73,15 @@ public interface StoreContext { */ public FetchConfiguration pushFetchConfiguration(); + /** + * Pushes the fetch configuration argument onto a stack, and makes the new configuration + * the active one. + * + * @since 2.1.1 + * @return the new fetch configuration + */ + public FetchConfiguration pushFetchConfiguration(FetchConfiguration fc); + /** * Pops the fetch configuration from the top of the stack, making the * next one down the active one. This returns void to avoid confusion, diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/AbstractPersistenceTestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/AbstractPersistenceTestCase.java index ce2e03aab..c892cbeb0 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/AbstractPersistenceTestCase.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/AbstractPersistenceTestCase.java @@ -271,6 +271,12 @@ public abstract class AbstractPersistenceTestCase extends TestCase { for (Broker b : ((AbstractBrokerFactory) JPAFacadeHelper.toBrokerFactory(emf)).getOpenBrokers()) { if (b != null && !b.isClosed()) { EntityManager em = JPAFacadeHelper.toEntityManager(b); + if( em.getTransaction().isActive() ) { + try { + em.getTransaction().rollback(); + } catch (Exception e) { + } + } closeEM(em); } } diff --git a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/SequencedActionsTest.java b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/SequencedActionsTest.java index 148ebb03d..7a8328fcc 100644 --- a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/SequencedActionsTest.java +++ b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/SequencedActionsTest.java @@ -684,41 +684,93 @@ public abstract class SequencedActionsTest extends SQLListenerTestCase { } } } - String testExClass = null; - Throwable curThrowable = null; int threadId = threadToRun; if (args.length > 1) { threadId = (Integer) args[1]; } - TestThread exThread = threads.get(threadId); - curThrowable = exThread.throwable; - testExClass = processException(curAction, curThrowable); + if( threadId != -1 ) { + // test exception on a specific thread + String testExClass = null; + Throwable curThrowable = null; + boolean exMatched = false; + TestThread exThread = threads.get(threadId); + curThrowable = exThread.throwable; + testExClass = processException(exThread, curAction, curThrowable); - boolean exMatched = false; - if (expectedExceptions != null - && expectedExceptions.size() > 0) { - for (Class expectedException : - expectedExceptions) { - if (matchExpectedException(curAct, expectedException, - curThrowable)) { + if (expectedExceptions != null + && expectedExceptions.size() > 0) { + for (Class expectedException : + expectedExceptions) { + if (matchExpectedException(curAct, expectedException, + curThrowable)) { + exMatched = true; + break; + } + } + } else { + if (curThrowable == null) { exMatched = true; - break; } } + if (!exMatched) { + log.trace(testExClass); + if (curThrowable != null) { + logStack(curThrowable); + } + } + assertTrue(curAct + ":Expecting=" + expectedExceptions + + ", Testing=" + testExClass, exMatched); + exThread.throwable = null; } else { - if (curThrowable == null) { - exMatched = true; - } + // test exception in any thread; used for deadlock exception testing since db server + // decides on which thread to terminate if deadlock is detected. + if (expectedExceptions == null || expectedExceptions.size() == 0) { + // Expecting no exception in all threads. + boolean noExMatched = true; + String aTestExClass = "["; + for (TestThread aThread : threads) { + Throwable aThrowable = aThread.throwable; + aTestExClass += processException(aThread, curAction, aThrowable) + ", "; + if (aThrowable != null) { + noExMatched = false; + log.trace(aTestExClass); + logStack(aThrowable); + aThread.throwable = null; + } + } + assertTrue(curAct + ":Expecting=[no exception]" + + ", Testing=" + aTestExClass + ']', noExMatched); + } else { + // Expecting any exception in any threads. + boolean aMatched = false; + String aTestExClass = "["; + for (TestThread aThread : threads) { + Throwable aThrowable = aThread.throwable; + aTestExClass += processException(aThread, curAction, aThrowable) + ", "; + + for (Class anExpectedException : expectedExceptions) { + if (matchExpectedException(curAct, + anExpectedException, aThrowable)) { + aMatched = true; + break; + } + } + if (aMatched) { + break; + } else { + if (aThrowable != null) { + logStack(aThrowable); + aThread.throwable = null; + } + } + } + if (!aMatched) { + log.trace(aTestExClass); + } + assertTrue(curAct + ":Expecting=" + expectedExceptions + + ", Testing=" + aTestExClass + "]", aMatched); + } } - if (!exMatched) { - log.trace(testExClass); - if (curThrowable != null) { - logStack(curThrowable); - } - } - assertTrue(curAct + ":Expecting=" + expectedExceptions - + ", Testing=" + testExClass, exMatched); - exThread.throwable = null; break; case WaitAllChildren: @@ -855,7 +907,7 @@ public abstract class SequencedActionsTest extends SQLListenerTestCase { log.trace("finally: commit completed"); } } catch(Exception finalEx) { - String failStr = processException(curAction, finalEx); + String failStr = processException(thisThread, curAction, finalEx); log.trace("Fincally:" + failStr); } } @@ -881,11 +933,11 @@ public abstract class SequencedActionsTest extends SQLListenerTestCase { return lockMode; } - private String processException(Act curAction, Throwable t) { - String failStr = "Caught exception: none"; + private String processException(TestThread thread, Act curAction, Throwable t) { + String failStr = "[" + thread.threadToRun + "] Caught exception: none"; if (t != null) { getLog().trace( - "Caught exception: " + t.getClass().getName() + ":" + t); + "[" + thread.threadToRun + "] Caught exception: " + t.getClass().getName() + ":" + t); logStack(t); Throwable rootCause = t.getCause(); failStr = "Failed on action '" + curAction + "' with exception " diff --git a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestMixedLockManagerDeadlock.java b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestMixedLockManagerDeadlock.java index 11ebae8e2..037e35207 100644 --- a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestMixedLockManagerDeadlock.java +++ b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestMixedLockManagerDeadlock.java @@ -19,6 +19,7 @@ package org.apache.openjpa.persistence.lockmgr; import java.util.Arrays; +import java.util.HashMap; import javax.persistence.EntityManager; import javax.persistence.LockModeType; @@ -28,12 +29,14 @@ import javax.persistence.LockModeType; */ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { private DBType dbType; + private HashMap[]> expWriteLockExClasses; public void setUp() { setSupportedDatabases( + org.apache.openjpa.jdbc.sql.DB2Dictionary.class, org.apache.openjpa.jdbc.sql.DerbyDictionary.class, org.apache.openjpa.jdbc.sql.OracleDictionary.class, - org.apache.openjpa.jdbc.sql.DB2Dictionary.class); + org.apache.openjpa.jdbc.sql.SQLServerDictionary.class); if (isTestsDisabled()) { return; } @@ -41,6 +44,12 @@ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { setUp(LockEmployee.class , "openjpa.LockManager", "mixed" ); + expWriteLockExClasses = new HashMap[]>(); + expWriteLockExClasses.put(DBType.db2, null); + expWriteLockExClasses.put(DBType.derby, ExpectingOptimisticLockExClass); + expWriteLockExClasses.put(DBType.oracle, null); + expWriteLockExClasses.put(DBType.sqlserver, ExpectingOptimisticLockExClass); + commonSetUp(); EntityManager em = emf.createEntityManager(); dbType = getDBType(em); @@ -49,8 +58,7 @@ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { /* ======== Find dead lock exception test ============*/ public void testFindDeadLockException() { commonFindTest("testFindDeadLockException", LockModeType.READ, null); - commonFindTest("testFindDeadLockException", LockModeType.WRITE, dbType == DBType.oracle ? null - : ExpectingOptimisticLockExClass); + commonFindTest("testFindDeadLockException", LockModeType.WRITE, expWriteLockExClasses.get(dbType)); commonFindTest("testFindDeadLockException", LockModeType.PESSIMISTIC_WRITE, ExpectingAnyLockExClass); } @@ -73,7 +81,7 @@ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { {Act.FindWithLock, 2, t1Lock}, {Act.WaitAllChildren}, - {Act.TestException, 1, t1Exceptions}, + {Act.TestException, -1, t1Exceptions}, // test t1Exceptions in any thread {Act.RollbackTx} }; Object[][] thread1 = { @@ -94,10 +102,8 @@ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { /* ======== named query dead lock exception test ============*/ public void testNamedQueryDeadLockException() { commonNamedQueryTest("testNamedQueryDeadLockException", LockModeType.READ, null); - commonNamedQueryTest("testNamedQueryDeadLockException", LockModeType.WRITE, dbType == DBType.oracle ? null - : ExpectingOptimisticLockExClass); -// commonNamedQueryTest("testNamedQueryDeadLockException", LockModeType.PESSIMISTIC_FORCE_INCREMENT, -// ExpectingAnyLockExClass); + commonNamedQueryTest("testNamedQueryDeadLockException", LockModeType.WRITE, expWriteLockExClasses.get(dbType)); + commonNamedQueryTest("testNamedQueryDeadLockException", LockModeType.PESSIMISTIC_FORCE_INCREMENT, ExpectingAnyLockExClass); } private void commonNamedQueryTest( String testName, @@ -119,7 +125,7 @@ public class TestMixedLockManagerDeadlock extends SequencedActionsTest { {Act.NamedQueryWithLock, "findEmployeeById", 2, t1Lock, "openjpa.hint.IgnorePreparedQuery", true}, {Act.WaitAllChildren}, - {Act.TestException, 1, t1Exceptions}, + {Act.TestException, -1, t1Exceptions}, {Act.RollbackTx}, {Act.CloseEm} diff --git a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java index 9f99cf9cd..a74ac6849 100644 --- a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java +++ b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java @@ -137,10 +137,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { em2.find(Employee.class, 2, LockModeType.PESSIMISTIC_READ, hints); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); } catch (Throwable e) { - if (!dict.supportsLockingWithMultipleTables) - assertError(e, PessimisticLockException.class); - else - assertError(e, LockTimeoutException.class); + assertError(e, PessimisticLockException.class, LockTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -206,10 +203,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { em2.find(Employee.class, 2, LockModeType.PESSIMISTIC_READ, map); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); } catch (Exception e) { - if (!dict.supportsLockingWithMultipleTables) - assertError(e, PessimisticLockException.class); - else - assertError(e, LockTimeoutException.class); + assertError(e, PessimisticLockException.class, LockTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -276,10 +270,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { assertTrue("Test department name = 'D20'", q.get(0).getName().equals("D10") || q.get(0).getName().equals("D20")); } catch (Exception ex) { - if (!dict.supportsLockingWithMultipleTables) - fail("Caught unexpected " + ex.getClass().getName() + ":" + ex.getMessage()); - else - assertError(ex, QueryTimeoutException.class); + assertError(ex, QueryTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -304,10 +295,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { List q = query.getResultList(); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); } catch (Exception e) { - if (!dict.supportsLockingWithMultipleTables) - assertError(e, PessimisticLockException.class); - else - assertError(e, QueryTimeoutException.class); + assertError(e, PessimisticLockException.class, QueryTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -342,10 +330,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { assertEquals("Expected 1 element with department id=20", q.size(), 1); assertEquals("Test department name = 'D20'", q.get(0).getName(), "D20"); } catch (Exception ex) { - if (!dict.supportsLockingWithMultipleTables) - fail("Caught unexpected " + ex.getClass().getName() + ":" + ex.getMessage()); - else - assertError(ex, QueryTimeoutException.class); + assertError(ex, QueryTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -370,10 +355,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { List q = query.getResultList(); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); } catch (Exception e) { - if (!dict.supportsLockingWithMultipleTables) - assertError(e, PessimisticLockException.class); - else - assertError(e, QueryTimeoutException.class); + assertError(e, PessimisticLockException.class, QueryTimeoutException.class); } finally { if (em1.getTransaction().isActive()) em1.getTransaction().rollback(); @@ -505,12 +487,22 @@ public class TestPessimisticLocks extends SQLListenerTestCase { * @param expeceted * type of the exception */ - void assertError(Throwable actual, Class expected) { - if (!expected.isAssignableFrom(actual.getClass())) { - actual.printStackTrace(); - throw new AssertionFailedError(actual.getClass().getName() + " was raised but expected " - + expected.getName()); - } + void assertError(Throwable actual, Class ... expected) { + boolean matched = false; + String expectedNames = ""; + for (Class aExpected : expected) { + expectedNames += aExpected.getName() + ", "; + if (aExpected.isAssignableFrom(actual.getClass())) { + matched = true; + } + } + if (!matched) { + actual.printStackTrace(); + throw new AssertionFailedError(actual.getClass().getName() + + " was raised but expecting one of the following: [" + + expectedNames.substring(0, expectedNames.length() - 2) + "]"); + } + Object failed = getFailedObject(actual); assertNotNull("Failed object is null", failed); assertNotEquals("null", failed); diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java index 18e39c930..3bc1fad19 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java @@ -163,10 +163,14 @@ public class EntityManagerImpl } public FetchPlan pushFetchPlan() { + return pushFetchPlan(null); + } + + public FetchPlan pushFetchPlan(FetchConfiguration fc) { assertNotCloseInvoked(); _broker.lock(); try { - _broker.pushFetchConfiguration(); + _broker.pushFetchConfiguration(fc); return getFetchPlan(); } finally { _broker.unlock(); diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java index 4dc8d3794..e2ec97ff7 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java @@ -316,22 +316,26 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { public List getResultList() { _em.assertNotCloseInvoked(); - Object ob = execute(); - if (ob instanceof List) { - List ret = (List) ob; - if (ret instanceof ResultList) { - RuntimeExceptionTranslator trans = PersistenceExceptions.getRollbackTranslator(_em); - if (_query.isDistinct()) { - return new DistinctResultList((ResultList) ret, trans); + boolean queryFetchPlanUsed = pushQueryFetchPlan(); + try { + Object ob = execute(); + if (ob instanceof List) { + List ret = (List) ob; + if (ret instanceof ResultList) { + RuntimeExceptionTranslator trans = PersistenceExceptions.getRollbackTranslator(_em); + if (_query.isDistinct()) { + return new DistinctResultList((ResultList) ret, trans); + } else { + return new DelegatingResultList((ResultList) ret, trans); + } } else { - return new DelegatingResultList((ResultList) ret, trans); + return ret; } - } else { - return ret; - } + } + return Collections.singletonList(ob); + } finally { + popQueryFetchPlan(queryFetchPlanUsed); } - - return Collections.singletonList(ob); } /** @@ -340,18 +344,45 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { public X getSingleResult() { _em.assertNotCloseInvoked(); setHint(QueryHints.HINT_RESULT_COUNT, 1); // for DB2 optimization - List result = getResultList(); - if (result == null || result.isEmpty()) - throw new NoResultException(_loc.get("no-result", getQueryString()) - .getMessage()); - if (result.size() > 1) - throw new NonUniqueResultException(_loc.get("non-unique-result", - getQueryString(), result.size()).getMessage()); + boolean queryFetchPlanUsed = pushQueryFetchPlan(); try { - return (X)result.get(0); - } catch (Exception e) { - throw new NoResultException(_loc.get("no-result", getQueryString()) - .getMessage()); + List result = getResultList(); + if (result == null || result.isEmpty()) + throw new NoResultException(_loc.get("no-result", getQueryString()) + .getMessage()); + if (result.size() > 1) + throw new NonUniqueResultException(_loc.get("non-unique-result", + getQueryString(), result.size()).getMessage()); + try { + return (X)result.get(0); + } catch (Exception e) { + throw new NoResultException(_loc.get("no-result", getQueryString()) + .getMessage()); + } + } finally { + popQueryFetchPlan(queryFetchPlanUsed); + } + } + + private boolean pushQueryFetchPlan() { + boolean fcPushed = false; + if (_fetch != null && _hintHandler != null) { + switch (_fetch.getReadLockMode()) { + case PESSIMISTIC_READ: + case PESSIMISTIC_WRITE: + case PESSIMISTIC_FORCE_INCREMENT: + // push query fetch plan to em if pessisimistic lock and any + // hints are set + _em.pushFetchPlan(((FetchPlanImpl)_fetch).getDelegate()); + fcPushed = true; + } + } + return fcPushed; + } + + private void popQueryFetchPlan(boolean queryFetchPlanUsed) { + if (queryFetchPlanUsed) { + _em.popFetchPlan(); } }