From 0c0d440329e31e43225fecb4a4df405ef2491eac Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Fri, 27 Aug 2010 16:52:52 +0000 Subject: [PATCH] ALF-4476 - Make transfer definitions for replication execution read only (locked) Also improve the testing of cancelling running replication jobs, and the debug output of the action tracking service git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22055 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org/alfresco/repo/action/ActionImpl.java | 1 + .../action/ActionTrackingServiceImpl.java | 19 ++++++-- .../ReplicationActionExecutor.java | 19 ++++++-- .../ReplicationServiceIntegrationTest.java | 43 +++++++++++++------ 4 files changed, 62 insertions(+), 20 deletions(-) diff --git a/source/java/org/alfresco/repo/action/ActionImpl.java b/source/java/org/alfresco/repo/action/ActionImpl.java index 5f8bcd953a..b16677b27a 100644 --- a/source/java/org/alfresco/repo/action/ActionImpl.java +++ b/source/java/org/alfresco/repo/action/ActionImpl.java @@ -187,6 +187,7 @@ public class ActionImpl extends ParameterizedItemImpl implements Action if (action instanceof ActionImpl) { ActionImpl actionImpl = (ActionImpl) action; + this.executionInstance = actionImpl.getExecutionInstance(); this.runAsUserName = actionImpl.getRunAsUser(); this.actionChain = actionImpl.actionChain; } diff --git a/source/java/org/alfresco/repo/action/ActionTrackingServiceImpl.java b/source/java/org/alfresco/repo/action/ActionTrackingServiceImpl.java index 4ce254d496..0d777b96a9 100644 --- a/source/java/org/alfresco/repo/action/ActionTrackingServiceImpl.java +++ b/source/java/org/alfresco/repo/action/ActionTrackingServiceImpl.java @@ -154,7 +154,7 @@ public class ActionTrackingServiceImpl implements ActionTrackingService { if (logger.isDebugEnabled() == true) { - logger.debug("Action " + action + " has begun exection"); + logger.debug("Action " + action + " with provisional key " + generateCacheKey(action) + " has begun exection"); } // Grab what status it was before @@ -179,7 +179,8 @@ public class ActionTrackingServiceImpl implements ActionTrackingService if(details == null) { logger.warn( "Went to mark the start of execution of " + - action + " but it wasn't in the running actions cache! " + + action + " with key " + key + + " but it wasn't in the running actions cache! " + "Your running actions cache is probably too small" ); } @@ -227,6 +228,11 @@ public class ActionTrackingServiceImpl implements ActionTrackingService // Put it into the cache ExecutionDetails details = buildExecutionDetails(action); executingActionsCache.put(key, details); + + if (logger.isDebugEnabled() == true) + { + logger.debug("Action " + action + " with key " + key + " placed into execution cache"); + } } /** @@ -332,10 +338,15 @@ public class ActionTrackingServiceImpl implements ActionTrackingService String key = generateCacheKey(action); ExecutionDetails details = getExecutionDetails(buildExecutionSummary(key)); if(details == null) { + Exception e = new Exception("Cancellation status missing from cache"); + e.fillInStackTrace(); + logger.warn( "Unable to check cancellation status for running action " + - action + " as it wasn't in the running actions cache! " + - "Your running actions cache is probably too small" + action + " with execution key " + key + + " as it wasn't in the running actions cache! " + + "Your running actions cache is probably too small", + e ); // Re-generate diff --git a/source/java/org/alfresco/repo/replication/ReplicationActionExecutor.java b/source/java/org/alfresco/repo/replication/ReplicationActionExecutor.java index fd2fadb04a..57ef0db34b 100644 --- a/source/java/org/alfresco/repo/replication/ReplicationActionExecutor.java +++ b/source/java/org/alfresco/repo/replication/ReplicationActionExecutor.java @@ -162,17 +162,21 @@ public class ReplicationActionExecutor extends ActionExecuterAbstractBase { new TransferDefinition(); transferDefinition.setNodes(toTransfer); transferDefinition.setSync(true); -// transferDefinition.setReadOnly(true); // TODO Make read only, but then need to fix tests + transferDefinition.setReadOnly(true); return transferDefinition; } @Override protected void executeImpl(Action action, NodeRef actionedUponNodeRef) { - // Specialise the action if needed, eg when loaded directly from - // the NodeRef without going via the replication service - if(action.getActionDefinitionName().equals(ReplicationDefinitionImpl.EXECUTOR_NAME)) + if(action instanceof ReplicationDefinition) { + // Already of the correct type + } + else if(action.getActionDefinitionName().equals(ReplicationDefinitionImpl.EXECUTOR_NAME)) + { + // Specialise the action if needed, eg when loaded directly from + // the NodeRef without going via the replication service action = new ReplicationDefinitionImpl(action); } @@ -294,6 +298,7 @@ public class ReplicationActionExecutor extends ActionExecuterAbstractBase { if(transferId != null) { transferService.cancelAsync(transferId); + logger.debug("Replication cancel was requested for " + replicationDef.getReplicationQName()); } else { @@ -331,6 +336,12 @@ public class ReplicationActionExecutor extends ActionExecuterAbstractBase { 6 // 6 times = wait up to 30 seconds ); } catch(LockAcquisitionException e) { + logger.debug( + "Unable to get the replication job lock on " + + replicationDef.getReplicationQName() + + ", retrying every " + (int)(retryTime/1000) + " seconds" + ); + // Long try - every 30 seconds lockToken = jobLockService.getLock( replicationDef.getReplicationQName(), diff --git a/source/java/org/alfresco/repo/replication/ReplicationServiceIntegrationTest.java b/source/java/org/alfresco/repo/replication/ReplicationServiceIntegrationTest.java index d112f10ff6..e591b1c116 100644 --- a/source/java/org/alfresco/repo/replication/ReplicationServiceIntegrationTest.java +++ b/source/java/org/alfresco/repo/replication/ReplicationServiceIntegrationTest.java @@ -181,7 +181,11 @@ public class ReplicationServiceIntegrationTest extends TestCase if(folder2 != null) { nodeService.deleteNode(folder2); } + + // Zap the destination folder, which may well contain + // entries transfered over which are locked if(destinationFolder != null) { + lockService.unlock(destinationFolder, true); nodeService.deleteNode(destinationFolder); } @@ -598,6 +602,10 @@ public class ReplicationServiceIntegrationTest extends TestCase * Check that cancelling works. * Does this by taking a lock on the job, cancelling, * releasing and seeing it abort. + * + * Tests that when we ask for a replication task to be cancelled, + * that it starts, cancels, and the status is correctly recorded + * for it. */ public void testReplicationExectionCancelling() throws Exception { @@ -623,16 +631,32 @@ public class ReplicationServiceIntegrationTest extends TestCase txn.begin(); actionService.executeAction(rd, replicationRoot, false, true); assertEquals(ActionStatus.Pending, rd.getExecutionStatus()); + + assertEquals(false, actionTrackingService.isCancellationRequested(rd)); + actionTrackingService.requestActionCancellation(rd); + assertEquals(true, actionTrackingService.isCancellationRequested(rd)); + txn.commit(); - - + // Let it get going, will be waiting for the lock // having registered with the action tracking service - Thread.sleep(500); + for(int i=0; i<100; i++) { + // Keep asking for it to be cancelled ASAP + actionTrackingService.requestActionCancellation(rd); + + if(rd.getExecutionStatus().equals(ActionStatus.Running)) { + // Good, has started up + // Stop waiting and do the cancel + break; + } else { + // Still pending, wait a bit more + Thread.sleep(10); + } + } + + // Ensure it started, and should shortly stop assertEquals(ActionStatus.Running, rd.getExecutionStatus()); - - // Now request the cancel - actionTrackingService.requestActionCancellation(rd); + assertEquals(true, actionTrackingService.isCancellationRequested(rd)); // Release our lock, should allow the replication task // to get going and spot the cancel @@ -856,7 +880,7 @@ public class ReplicationServiceIntegrationTest extends TestCase TransferDefinition td = replicationActionExecutor.buildTransferDefinition(rd, nodes); assertEquals(true, td.isSync()); -// assertEquals(true, td.isReadOnly());// TODO Make read only, and fix tests + assertEquals(true, td.isReadOnly()); assertEquals(2, td.getNodes().size()); assertEquals(true, td.getNodes().contains(folder1)); assertEquals(true, td.getNodes().contains(content1_1)); @@ -1012,11 +1036,6 @@ public class ReplicationServiceIntegrationTest extends TestCase // TODO } - public void testCancellation() throws Exception - { - // TODO - } - private NodeRef makeNode(NodeRef parent, QName nodeType) {