diff --git a/source/java/org/alfresco/repo/node/archive/NodeArchiveServiceImpl.java b/source/java/org/alfresco/repo/node/archive/NodeArchiveServiceImpl.java index 5c38d933bb..f3cec7a42f 100644 --- a/source/java/org/alfresco/repo/node/archive/NodeArchiveServiceImpl.java +++ b/source/java/org/alfresco/repo/node/archive/NodeArchiveServiceImpl.java @@ -30,8 +30,8 @@ import java.util.List; import org.alfresco.model.ContentModel; import org.alfresco.repo.node.archive.RestoreNodeReport.RestoreStatus; import org.alfresco.repo.security.permissions.AccessDeniedException; -import org.alfresco.repo.transaction.TransactionUtil; -import org.alfresco.repo.transaction.TransactionUtil.TransactionWork; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.InvalidNodeRefException; import org.alfresco.service.cmr.repository.NodeRef; @@ -129,14 +129,15 @@ public class NodeArchiveServiceImpl implements NodeArchiveService try { // Transactional wrapper to attempt the restore - TransactionWork restoreWork = new TransactionWork() + RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper(); + RetryingTransactionCallback restoreCallback = new RetryingTransactionCallback() { - public NodeRef doWork() throws Exception + public NodeRef execute() throws Exception { return nodeService.restoreNode(archivedNodeRef, destinationNodeRef, assocTypeQName, assocQName); } }; - NodeRef newNodeRef = TransactionUtil.executeInNonPropagatingUserTransaction(transactionService, restoreWork); + NodeRef newNodeRef = txnHelper.doInTransaction(restoreCallback, false, true); // success report.setRestoredNodeRef(newNodeRef); report.setStatus(RestoreStatus.SUCCESS); @@ -274,9 +275,10 @@ public class NodeArchiveServiceImpl implements NodeArchiveService */ public void purgeArchivedNode(final NodeRef archivedNodeRef) { - TransactionWork deleteWork = new TransactionWork() + RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper(); + RetryingTransactionCallback deleteCallback = new RetryingTransactionCallback() { - public Object doWork() throws Exception + public Object execute() throws Exception { try { @@ -289,7 +291,7 @@ public class NodeArchiveServiceImpl implements NodeArchiveService return null; } }; - TransactionUtil.executeInNonPropagatingUserTransaction(transactionService, deleteWork); + txnHelper.doInTransaction(deleteCallback, false, true); } /** diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java index 598c7bdc6d..a294c8e5af 100644 --- a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java +++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java @@ -27,7 +27,6 @@ package org.alfresco.repo.transaction; import java.sql.BatchUpdateException; import java.util.Random; -import javax.transaction.RollbackException; import javax.transaction.Status; import javax.transaction.SystemException; import javax.transaction.UserTransaction; @@ -57,7 +56,7 @@ public class RetryingTransactionHelper /** * Exceptions that trigger retries. */ - private static final Class[] RETRY_EXCEPTIONS; + public static final Class[] RETRY_EXCEPTIONS; static { RETRY_EXCEPTIONS = new Class[] { @@ -149,6 +148,7 @@ public class RetryingTransactionHelper * * @param cb The callback containing the unit of work. * @return Returns the result of the unit of work. + * @throws RuntimeException all checked exceptions are converted */ public R doInTransaction(RetryingTransactionCallback cb) { @@ -167,6 +167,7 @@ public class RetryingTransactionHelper * @param cb The callback containing the unit of work. * @param readOnly Whether this is a read only transaction. * @return Returns the result of the unit of work. + * @throws RuntimeException all checked exceptions are converted */ public R doInTransaction(RetryingTransactionCallback cb, boolean readOnly) { @@ -184,11 +185,12 @@ public class RetryingTransactionHelper * * @param cb The callback containing the unit of work. * @param readOnly Whether this is a read only transaction. - * @param newTransaction true to force a new transaction or + * @param requiresNew true to force a new transaction or * false to partake in any existing transaction. * @return Returns the result of the unit of work. + * @throws RuntimeException all checked exceptions are converted */ - public R doInTransaction(RetryingTransactionCallback cb, boolean readOnly, boolean newTransaction) + public R doInTransaction(RetryingTransactionCallback cb, boolean readOnly, boolean requiresNew) { if (this.readOnly && !readOnly) { @@ -203,7 +205,7 @@ public class RetryingTransactionHelper boolean isNew = false; try { - if (newTransaction) + if (requiresNew) { txn = fTxnService.getNonPropagatingUserTransaction(); } @@ -211,12 +213,10 @@ public class RetryingTransactionHelper { txn = fTxnService.getUserTransaction(readOnly); } - // Do we need to handle transaction demarcation. If no, we cannot do retries, - // that will be up to the containing transaction. - isNew = newTransaction || txn.getStatus() == Status.STATUS_NO_TRANSACTION; // Only start a transaction if required. This check isn't necessary as the transactional // behaviour ensures that the appropriate propogation is performed. It is a useful and // simple optimization. + isNew = requiresNew || txn.getStatus() == Status.STATUS_NO_TRANSACTION; if (isNew) { txn.begin(); @@ -250,13 +250,6 @@ public class RetryingTransactionHelper } return result; } - catch (RollbackException e) - { - // Our explicit rollback didn't work - throw new AlfrescoRuntimeException( - "Unexpected rollback exception: \n" + e.getMessage(), - e); - } catch (Throwable e) { // Somebody else 'owns' the transaction, so just rethrow. @@ -268,7 +261,9 @@ public class RetryingTransactionHelper } else { - throw new AlfrescoRuntimeException("Unknown Exception.", e); + throw new AlfrescoRuntimeException( + "Exception from transactional callback: " + cb, + e); } } // Rollback if we can. @@ -283,15 +278,15 @@ public class RetryingTransactionHelper } catch (IllegalStateException e1) { - throw new AlfrescoRuntimeException("Failure during rollback.", e1); + throw new AlfrescoRuntimeException("Failure during rollback: " + cb, e1); } catch (SecurityException e1) { - throw new AlfrescoRuntimeException("Failure during rollback.", e1); + throw new AlfrescoRuntimeException("Failure during rollback: " + cb, e1); } catch (SystemException e1) { - throw new AlfrescoRuntimeException("Failure during rollback.", e1); + throw new AlfrescoRuntimeException("Failure during rollback: " + cb, e1); } } lastException = (e instanceof RuntimeException) ? diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java new file mode 100644 index 0000000000..2a346409a0 --- /dev/null +++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java @@ -0,0 +1,370 @@ +/* + * Copyright (C) 2005-2007 Alfresco Software Limited. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + * As a special exception to the terms and conditions of version 2.0 of + * the GPL, you may redistribute this Program in connection with Free/Libre + * and Open Source Software ("FLOSS") applications as described in Alfresco's + * FLOSS exception. You should have recieved a copy of the text describing + * the FLOSS exception, and it is also available here: + * http://www.alfresco.com/legal/licensing" + */ +package org.alfresco.repo.transaction; + +import junit.framework.TestCase; + +import org.alfresco.error.ExceptionStackUtil; +import org.alfresco.model.ContentModel; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; +import org.alfresco.service.cmr.repository.InvalidNodeRefException; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.cmr.repository.StoreRef; +import org.alfresco.service.namespace.NamespaceService; +import org.alfresco.service.namespace.QName; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.springframework.context.ApplicationContext; +import org.springframework.dao.ConcurrencyFailureException; + +/** + * Tests the transaction retrying behaviour with various failure modes. + * + * @see RetryingTransactionHelper + * @see TransactionService + * + * @author Derek Hulley + * @since 2.1 + */ +public class RetryingTransactionHelperTest extends TestCase +{ + private static final QName PROP_CHECK_VALUE = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "check_value"); + + private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); + + private ServiceRegistry serviceRegistry; + private TransactionService transactionService; + private NodeService nodeService; + private RetryingTransactionHelper txnHelper; + + private NodeRef rootNodeRef; + private NodeRef workingNodeRef; + + @Override + public void setUp() throws Exception + { + serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); + transactionService = serviceRegistry.getTransactionService(); + nodeService = serviceRegistry.getNodeService(); + txnHelper = transactionService.getRetryingTransactionHelper(); + + StoreRef storeRef = nodeService.createStore( + StoreRef.PROTOCOL_WORKSPACE, + "test-" + getName() + "-" + System.currentTimeMillis()); + rootNodeRef = nodeService.getRootNode(storeRef); + // Create a node to work on + workingNodeRef = nodeService.createNode( + rootNodeRef, + ContentModel.ASSOC_CHILDREN, + QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, getName()), + ContentModel.TYPE_CMOBJECT).getChildRef(); + } + + public void testSetUp() throws Exception + { + assertNotNull(rootNodeRef); + assertNotNull(workingNodeRef); + } + + /** + * Get the count, which is 0 to start each test + * + * @return Returns the current count + */ + private Long getCheckValue() + { + Long checkValue = (Long) nodeService.getProperty(workingNodeRef, PROP_CHECK_VALUE); + if (checkValue == null) + { + checkValue = new Long(0); + nodeService.setProperty(workingNodeRef, PROP_CHECK_VALUE, checkValue); + } + return checkValue; + } + + /** + * Increment the test count, which is 0 to start each test + * + * @return Returns the current count + */ + private Long incrementCheckValue() + { + Long checkValue = getCheckValue(); + checkValue = new Long(checkValue.longValue() + 1L); + nodeService.setProperty(workingNodeRef, PROP_CHECK_VALUE, checkValue); + return checkValue; + } + + /** + * @return Never returns anything + * @throws InvalidNodeRefException ALWAYS + */ + private Long blowUp() + { + NodeRef invalidNodeRef = new NodeRef(workingNodeRef.getStoreRef(), "BOGUS"); + nodeService.setProperty(invalidNodeRef, PROP_CHECK_VALUE, null); + fail("Expected to generate an InvalidNodeRefException"); + return null; + } + + /** + * Check that it works without complications. + */ + public void testSuccessNoRetry() + { + long beforeValue = getCheckValue(); + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + return incrementCheckValue(); + } + }; + long txnValue = txnHelper.doInTransaction(callback); + long afterValue = getCheckValue(); + assertEquals("The value must have increased", beforeValue + 1, afterValue); + assertEquals("The txn value must be the same as the value after", afterValue, txnValue); + } + + /** + * Check that the retries happening for simple concurrency exceptions + */ + public void testSuccessWithRetry() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + private int maxCalls = 3; + private int callCount = 0; + public Long execute() throws Throwable + { + callCount++; + Long checkValue = incrementCheckValue(); + if (callCount == maxCalls) + { + return checkValue; + } + else + { + throw new ConcurrencyFailureException("Testing"); + } + } + }; + long txnValue = txnHelper.doInTransaction(callback); + assertEquals("Only one increment expected", 1, txnValue); + } + + /** + * Checks that a non-retrying exception is passed out and that the transaction is rolled back. + */ + public void testNonRetryingFailure() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + incrementCheckValue(); + return blowUp(); + } + }; + try + { + txnHelper.doInTransaction(callback); + fail("Wrapper didn't generate an exception"); + } + catch (InvalidNodeRefException e) + { + // Correct + } + catch (Throwable e) + { + fail("Incorrect exception from wrapper: " + e); + } + // Check that the value didn't change + long checkValue = getCheckValue(); + assertEquals("Check value should not have changed", 0, checkValue); + } + + /** + * Sometimes, exceptions or other states may cause the transaction to be marked for + * rollback without an exception being generated. This tests that the exception stays + * absorbed and that another isn't generated, but that the transaction was rolled back + * properly. + */ + public void testNonRetryingSilentRollback() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + incrementCheckValue(); + try + { + return blowUp(); + } + catch (InvalidNodeRefException e) + { + // Expected, but absorbed + } + return null; + } + }; + txnHelper.doInTransaction(callback); + long checkValue = getCheckValue(); + assertEquals("Check value should not have changed", 0, checkValue); + } + + /** + * Checks nesting of two transactions with requiresNew == false + */ + public void testNestedWithPropogation() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + RetryingTransactionCallback callbackInner = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + incrementCheckValue(); + incrementCheckValue(); + return getCheckValue(); + } + }; + txnHelper.doInTransaction(callbackInner, false, false); + incrementCheckValue(); + incrementCheckValue(); + return getCheckValue(); + } + }; + long checkValue = txnHelper.doInTransaction(callback); + assertEquals("Nesting requiresNew==false didn't work", 4, checkValue); + } + + /** + * Checks nesting of two transactions with requiresNew == true + */ + public void testNestedWithoutPropogation() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + RetryingTransactionCallback callbackInner = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + incrementCheckValue(); + incrementCheckValue(); + return getCheckValue(); + } + }; + txnHelper.doInTransaction(callbackInner, false, true); + incrementCheckValue(); + incrementCheckValue(); + return getCheckValue(); + } + }; + long checkValue = txnHelper.doInTransaction(callback); + assertEquals("Nesting requiresNew==true didn't work", 4, checkValue); + } + + /** + * Checks nesting of two transactions with requiresNew == true, + * but where the two transactions get involved in a concurrency struggle. + */ + public void testNestedWithoutPropogationConcurrentUntilFailure() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + RetryingTransactionCallback callbackInner = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + incrementCheckValue(); + return getCheckValue(); + } + }; + incrementCheckValue(); + txnHelper.doInTransaction(callbackInner, false, true); + return getCheckValue(); + } + }; + try + { + txnHelper.doInTransaction(callback); + fail("Concurrent nested access not leading to failure"); + } + catch (Throwable e) + { + Throwable validCause = ExceptionStackUtil.getCause(e, RetryingTransactionHelper.RETRY_EXCEPTIONS); + assertNotNull("Unexpected cause of the failure", validCause); + } + } + + /** + * Checks nesting of two transactions with requiresNew == true, + * but where the inner transaction fails writes values that the outer transaction + * fails on. + */ + public void testNestedWithoutPropogationOuterFailing() + { + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + private int maxCalls = 3; + private int callCount = 0; + public Long execute() throws Throwable + { + callCount++; + RetryingTransactionCallback callbackInner = new RetryingTransactionCallback() + { + public Long execute() throws Throwable + { + for (int i = 0; i < 5; i++) + { + incrementCheckValue(); + } + return getCheckValue(); + } + }; + // Increment the value so that the outer transaction is bound to the particular + // version of the data + incrementCheckValue(); + // Don't execute the inner transaction the last time around + if (callCount < maxCalls) + { + txnHelper.doInTransaction(callbackInner, false, true); + } + return getCheckValue(); + } + }; + long checkValue = txnHelper.doInTransaction(callback); + assertEquals("Check value not incremented", 11, checkValue); + } +}