From 9e87100a6b65ebb256433cc9aa95700f3806d222 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Tue, 7 Sep 2010 11:09:20 +0000 Subject: [PATCH] More ALF-588: MT - delete tenant requires deleteStore - Reintroduced testDeleteStore - Added 'protocolsToIgnore' property to index recovery components - Added 'deleted' protocol to all ignorable store settings (ADM indexer, index recovery) - Return 'NodeRef.Status' for transaction changes queries (removes N+1 calls back to NodeService) git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22290 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/core-services-context.xml | 3 + .../node-common-SqlMap.xml | 15 +- config/alfresco/index-recovery-context.xml | 5 + .../org/alfresco/repo/avm/AVMNodeService.java | 2 +- .../repo/domain/node/AbstractNodeDAOImpl.java | 12 +- .../alfresco/repo/domain/node/NodeDAO.java | 8 +- .../alfresco/repo/domain/node/NodeEntity.java | 6 + .../repo/node/BaseNodeServiceTest.java | 9 +- .../node/index/AbstractReindexComponent.java | 193 +++++++----------- .../index/FullIndexRecoveryComponent.java | 12 +- .../index/FullIndexRecoveryComponentTest.java | 4 - 11 files changed, 117 insertions(+), 152 deletions(-) diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index a734cb4475..c3ae5806a9 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -528,6 +528,9 @@ + + + diff --git a/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml b/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml index 468fd46875..359771ef2b 100644 --- a/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml +++ b/config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml @@ -69,6 +69,15 @@ + + + + + + + + + @@ -893,13 +902,15 @@ ) - select node.id as id, store.protocol as protocol, store.identifier as identifier, node.uuid as uuid, - node.node_deleted as node_deleted + node.node_deleted as node_deleted, + txn.id as txn_id, + txn.change_txn_id as txn_change_id from alf_node node join alf_store store on (store.id = node.store_id) diff --git a/config/alfresco/index-recovery-context.xml b/config/alfresco/index-recovery-context.xml index 4f27fc81da..d95f8d23af 100644 --- a/config/alfresco/index-recovery-context.xml +++ b/config/alfresco/index-recovery-context.xml @@ -46,6 +46,11 @@ + + + deleted + + ${version.store.version2Store} diff --git a/source/java/org/alfresco/repo/avm/AVMNodeService.java b/source/java/org/alfresco/repo/avm/AVMNodeService.java index f2b00a4dd7..953ef8454d 100644 --- a/source/java/org/alfresco/repo/avm/AVMNodeService.java +++ b/source/java/org/alfresco/repo/avm/AVMNodeService.java @@ -306,7 +306,7 @@ public class AVMNodeService extends AbstractNodeServiceImpl implements NodeServi { // TODO Need to find out if this is important and if so // need to capture Transaction IDs. - return new NodeRef.Status("Unknown", null, !exists(nodeRef)); + return new NodeRef.Status(nodeRef, "Unknown", null, !exists(nodeRef)); } /** diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index 79e8276236..5d5210adef 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -775,7 +775,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO else { Transaction txn = node.getTransaction(); - return new NodeRef.Status(txn.getChangeTxnId(), txn.getId(), node.getDeleted()); + return new NodeRef.Status(nodeRef, txn.getChangeTxnId(), txn.getId(), node.getDeleted()); } } @@ -3053,23 +3053,23 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return selectTxnById(txnId); } - public List getTxnChanges(Long txnId) + public List getTxnChanges(Long txnId) { return getTxnChangesForStore(null, txnId); } - public List getTxnChangesForStore(StoreRef storeRef, Long txnId) + public List getTxnChangesForStore(StoreRef storeRef, Long txnId) { Long storeId = (storeRef == null) ? null : getStoreNotNull(storeRef).getId(); List nodes = selectTxnChanges(txnId, storeId); // Convert - List nodeRefs = new ArrayList(nodes.size()); + List nodeStatuses = new ArrayList(nodes.size()); for (NodeEntity node : nodes) { - nodeRefs.add(node.getNodeRef()); + nodeStatuses.add(node.getNodeStatus()); } // Done - return nodeRefs; + return nodeStatuses; } public int getTxnUpdateCount(Long txnId) diff --git a/source/java/org/alfresco/repo/domain/node/NodeDAO.java b/source/java/org/alfresco/repo/domain/node/NodeDAO.java index 130d4d3f22..e19aafdbd0 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeDAO.java +++ b/source/java/org/alfresco/repo/domain/node/NodeDAO.java @@ -593,14 +593,14 @@ public interface NodeDAO extends NodeBulkLoader public int getTransactionCount(); /** - * @deprecated Replace with query returning a Node object + * @return Returns the node statuses for a transaction, limited to the store */ - public List getTxnChangesForStore(StoreRef storeRef, Long txnId); + public List getTxnChangesForStore(StoreRef storeRef, Long txnId); /** - * @deprecated Replace with query returning a Node object + * @return Returns the node statuses for a transaction, regardless of store */ - public List getTxnChanges(Long txnId); + public List getTxnChanges(Long txnId); public List getTxnsUnused(Long minTxnId, long maxCommitTime, int count); diff --git a/source/java/org/alfresco/repo/domain/node/NodeEntity.java b/source/java/org/alfresco/repo/domain/node/NodeEntity.java index 938359cb2c..f4b0824009 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodeEntity.java @@ -140,6 +140,12 @@ public class NodeEntity implements Node return new NodeRef(store.getStoreRef(), uuid); } + public NodeRef.Status getNodeStatus() + { + NodeRef nodeRef = new NodeRef(store.getStoreRef(), uuid); + return new NodeRef.Status(nodeRef, transaction.getChangeTxnId(), transaction.getId(), deleted); + } + public Pair getNodePair() { return new Pair(id, getNodeRef()); diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java index d0846f194a..2afb58e88d 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java @@ -463,20 +463,17 @@ public abstract class BaseNodeServiceTest extends BaseSpringTest assertTrue("New store not present is list of stores", storeRefs.contains(storeRef)); } - /** - * TODO: Comment back in and fix up IndexCheckServiceTest - */ - public void xtestDeleteStore() throws Exception + public void testDeleteStore() throws Exception { StoreRef storeRef = createStore(); // get all stores List storeRefs = nodeService.getStores(); // check that the store ref is present - assertTrue("New store not present is list of stores", storeRefs.contains(storeRef)); + assertTrue("New store not present in list of stores", storeRefs.contains(storeRef)); // Delete it nodeService.deleteStore(storeRef); storeRefs = nodeService.getStores(); - assertFalse("Deleted store should not present is list of stores", storeRefs.contains(storeRef)); + assertFalse("Deleted store should not present in list of stores", storeRefs.contains(storeRef)); // Now make sure that none of the stores have the "deleted" protocol for (StoreRef retrievedStoreRef : storeRefs) { diff --git a/source/java/org/alfresco/repo/node/index/AbstractReindexComponent.java b/source/java/org/alfresco/repo/node/index/AbstractReindexComponent.java index c5a56fd685..a4cbc923ce 100644 --- a/source/java/org/alfresco/repo/node/index/AbstractReindexComponent.java +++ b/source/java/org/alfresco/repo/node/index/AbstractReindexComponent.java @@ -20,9 +20,10 @@ package org.alfresco.repo.node.index; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; @@ -48,15 +49,14 @@ import org.alfresco.service.cmr.repository.ChildAssociationRef; 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.cmr.repository.NodeRef.Status; import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.search.SearchParameters; import org.alfresco.service.cmr.search.SearchService; +import org.alfresco.util.ParameterCheck; import org.alfresco.util.PropertyCheck; import org.alfresco.util.VmShutdownListener; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.extensions.surf.util.ParameterCheck; /** * Abstract helper for reindexing. @@ -91,7 +91,8 @@ public abstract class AbstractReindexComponent implements IndexRecovery private ThreadPoolExecutor threadPoolExecutor; private TenantService tenantService; - private List storesToIgnore = new ArrayList(0); + private Set storeProtocolsToIgnore = new HashSet(7); + private Set storesToIgnore = new HashSet(7); private volatile boolean shutdown; private final WriteLock indexerWriteLock; @@ -208,16 +209,44 @@ public abstract class AbstractReindexComponent implements IndexRecovery this.tenantService = tenantService; } - public void setStoresToIgnore(List storesToIgnore) + /** + * @param storeProtocolsToIgnore a list of store protocols that will be ignored + * by the index check code e.g. 'deleted' in 'deleted://MyStore' + * @since 3.4 + */ + public void setStoreProtocolsToIgnore(List storeProtocolsToIgnore) { - this.storesToIgnore = storesToIgnore; + for (String storeProtocolToIgnore : storeProtocolsToIgnore) + { + this.storeProtocolsToIgnore.add(storeProtocolToIgnore); + } } - public List getStoresToIgnore() + /** + * @param storesToIgnore a list of store references that will be ignored + * by the index check code e.g. 'test://TestOne' + */ + public void setStoresToIgnore(List storesToIgnore) { - return this.storesToIgnore; + for (String storeToIgnore : storesToIgnore) + { + StoreRef storeRef = new StoreRef(storeToIgnore); + this.storesToIgnore.add(storeRef); + } } - + + /** + * Find out if a store is ignored by the indexing code + * + * @param storeRef the store to check + * @return Returns true if the store reference provided is not indexed + */ + public boolean isIgnorableStore(StoreRef storeRef) + { + storeRef = tenantService.getBaseName(storeRef); // Convert to tenant-safe check + return storesToIgnore.contains(storeRef) || storeProtocolsToIgnore.contains(storeRef.getProtocol()); + } + /** * Determines if calls to {@link #reindexImpl()} should be wrapped in a transaction or not. * The default is true. @@ -242,13 +271,14 @@ public abstract class AbstractReindexComponent implements IndexRecovery */ public final void reindex() { -// PropertyCheck.mandatory(this, "authenticationComponent", this.authenticationComponent); PropertyCheck.mandatory(this, "ftsIndexer", this.ftsIndexer); PropertyCheck.mandatory(this, "indexer", this.indexer); PropertyCheck.mandatory(this, "searcher", this.searcher); PropertyCheck.mandatory(this, "nodeService", this.nodeService); PropertyCheck.mandatory(this, "nodeDaoService", this.nodeDAO); PropertyCheck.mandatory(this, "transactionComponent", this.transactionService); + PropertyCheck.mandatory(this, "storesToIgnore", this.storesToIgnore); + PropertyCheck.mandatory(this, "storeProtocolsToIgnore", this.storeProtocolsToIgnore); if (indexerWriteLock.tryLock()) { @@ -336,19 +366,14 @@ public abstract class AbstractReindexComponent implements IndexRecovery } } - List storesToIgnore = getStoresToIgnore(); - if (storesToIgnore != null) + storeRefsIterator = storeRefs.iterator(); + while (storeRefsIterator.hasNext()) { - - storeRefsIterator = storeRefs.iterator(); - while (storeRefsIterator.hasNext()) + // Remove stores to ignore + StoreRef storeRef = storeRefsIterator.next(); + if (isIgnorableStore(storeRef)) { - // Remove stores to ignore - StoreRef storeRef = storeRefsIterator.next(); - if (storesToIgnore.contains(storeRef.toString())) - { - storeRefsIterator.remove(); - } + storeRefsIterator.remove(); } } @@ -369,17 +394,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery return storeRefs; } -// Unused - comemnted out to make use clearer for isTxnPresentInIndex -// protected InIndex isTxnIdPresentInIndex(long txnId) -// { -// Transaction txn = nodeDaoService.getTxnById(txnId); -// if (txn == null) -// { -// return InIndex.YES; -// } -// return isTxnPresentInIndex(txn); -// } - /** * Determines if a given transaction is definitely in the index or not. * @@ -418,39 +432,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery // If none of the stores have the transaction, then that might be because it consists of 0 modifications int updateCount = nodeDAO.getTxnUpdateCount(txnId); - /* Alternative (r15360) - // exclude updates in the version store - if(updateCount > 0) - { - // the updates could all be in the version stores ... - List changes = nodeDaoService.getTxnChanges(txnId); - for(NodeRef change : changes) - { - StoreRef changeStore = change.getStoreRef(); - if(changeStore.getProtocol().equals(StoreRef.PROTOCOL_WORKSPACE)) - { - if(changeStore.getIdentifier().equals("lightWeightVersionStore")) - { - Status nodeStatus = nodeService.getNodeStatus(change); - if(!nodeStatus.isDeleted()) - { - updateCount--; - } - } - if(changeStore.getIdentifier().equals("version2Store")) - { - Status nodeStatus = nodeService.getNodeStatus(change); - if(!nodeStatus.isDeleted()) - { - updateCount--; - } - } - } - - } - } - */ - if ((updateCount > 0) && (! allUpdatedNodesCanBeIgnored(txnId))) { // There were updates, but there is no sign in the indexes @@ -543,38 +524,24 @@ public abstract class AbstractReindexComponent implements IndexRecovery protected boolean allUpdatedNodesCanBeIgnored(Long txnId) { + ParameterCheck.mandatory("txnId", txnId); + boolean allUpdatedNodesCanBeIgnored = false; - List storesToIgnore = getStoresToIgnore(); - if ((storesToIgnore != null) && (storesToIgnore.size() > 0) && (txnId != null)) + + List nodeStatuses = nodeDAO.getTxnChanges(txnId); + + allUpdatedNodesCanBeIgnored = true; + for (NodeRef.Status nodeStatus : nodeStatuses) { - List nodeRefs = nodeDAO.getTxnChanges(txnId); - - allUpdatedNodesCanBeIgnored = true; - for (NodeRef nodeRef : nodeRefs) + NodeRef nodeRef = nodeStatus.getNodeRef(); + if (! nodeStatus.isDeleted()) { - if (nodeRef != null) + // updated node (ie. not deleted) + StoreRef storeRef = nodeRef.getStoreRef(); + if (!isIgnorableStore(storeRef)) { - Status nodeStatus = nodeService.getNodeStatus(nodeRef); - if (nodeStatus == null) - { - // it's not there any more - continue; - } - if (! nodeStatus.isDeleted()) - { - // updated node (ie. not deleted) - StoreRef storeRef = nodeRef.getStoreRef(); - if (tenantService != null) - { - storeRef = tenantService.getBaseName(nodeRef.getStoreRef()); - } - - if (! storesToIgnore.contains(storeRef.toString())) - { - allUpdatedNodesCanBeIgnored = false; - break; - } - } + allUpdatedNodesCanBeIgnored = false; + break; } } } @@ -587,15 +554,18 @@ public abstract class AbstractReindexComponent implements IndexRecovery final Long txnId = txn.getId(); // there have been deletes, so we have to ensure that none of the nodes deleted are present in the index // get all node refs for the transaction - List nodeRefs = nodeDAO.getTxnChangesForStore(storeRef, txnId); + List nodeStatuses = nodeDAO.getTxnChangesForStore(storeRef, txnId); boolean foundNodeRef = false; - for (NodeRef nodeRef : nodeRefs) + for (NodeRef.Status nodeStatus : nodeStatuses) { + NodeRef nodeRef = nodeStatus.getNodeRef(); if (logger.isTraceEnabled()) { - logger.trace("Searching for node in index: \n" + + logger.trace( + + "Searching for node in index: \n" + " node: " + nodeRef + "\n" + - " txn: " + txnId); + " txn: " + txnId); } // we know that these are all deletions ResultSet results = null; @@ -689,12 +659,12 @@ public abstract class AbstractReindexComponent implements IndexRecovery } // get the node references pertinent to the transaction - List nodeRefs = nodeDAO.getTxnChanges(txnId); + List nodeStatuses = nodeDAO.getTxnChanges(txnId); // reindex each node int nodeCount = 0; - for (NodeRef nodeRef : nodeRefs) + for (NodeRef.Status nodeStatus : nodeStatuses) { - Status nodeStatus = nodeService.getNodeStatus(nodeRef); + NodeRef nodeRef = nodeStatus.getNodeRef(); if (nodeStatus == null) { // it's not there any more @@ -748,7 +718,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery private final List txnIds; private long lastIndexedTimestamp; private boolean atHeadOfQueue; - private boolean killed; private ReindexWorkerRunnable(List txnIds) { @@ -760,7 +729,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery this.uidHashCode = id * 13 + 11; this.txnIds = txnIds; this.atHeadOfQueue = false; - this.killed = false; recordTimestamp(); } @@ -792,16 +760,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery return uidHashCode; } - public synchronized void kill() - { - this.killed = true; - } - - private synchronized boolean isKilled() - { - return killed; - } - /** * @return the time that the last node was indexed (nanoseconds) */ @@ -912,11 +870,6 @@ public abstract class AbstractReindexComponent implements IndexRecovery public synchronized void reindexedNode(NodeRef nodeRef) { - // Check for forced kill - if (isKilled()) - { - throw new ReindexTerminatedException(); - } recordTimestamp(); } @@ -975,15 +928,15 @@ public abstract class AbstractReindexComponent implements IndexRecovery peek.setAtHeadOfQueue(); } // Check kill switch - if (peek == null || isKilled() || isAtHeadOfQueue()) + if (peek == null || isAtHeadOfQueue()) { // Going to close break; } else { - // This thread is not at the head of the queue and has not been flagged - // for death, so just wait until someone notifies us to carry on + // This thread is not at the head of the queue so just wait + // until someone notifies us to carry on waitForHeadOfQueue(); // Loop again } diff --git a/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponent.java b/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponent.java index 9fafadef4d..7468068016 100644 --- a/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponent.java +++ b/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponent.java @@ -31,7 +31,6 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransacti import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.StoreRef; -import org.alfresco.service.cmr.repository.NodeRef.Status; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.extensions.surf.util.I18NUtil; @@ -446,16 +445,11 @@ public class FullIndexRecoveryComponent extends AbstractReindexComponent public Object execute() throws Exception { // get the node references pertinent to the transaction - List nodeRefs = nodeDAO.getTxnChanges(txnId); + List nodeStatuses = nodeDAO.getTxnChanges(txnId); // reindex each node - for (NodeRef nodeRef : nodeRefs) + for (NodeRef.Status nodeStatus : nodeStatuses) { - Status nodeStatus = nodeService.getNodeStatus(nodeRef); - if (nodeStatus == null) - { - // it's not there any more - continue; - } + NodeRef nodeRef = nodeStatus.getNodeRef(); if (nodeStatus.isDeleted()) // node deleted { // only the child node ref is relevant diff --git a/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponentTest.java b/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponentTest.java index bcff946cb3..b9d5778ed8 100644 --- a/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponentTest.java +++ b/source/java/org/alfresco/repo/node/index/FullIndexRecoveryComponentTest.java @@ -67,10 +67,6 @@ public class FullIndexRecoveryComponentTest extends TestCase testTX = transactionService.getUserTransaction(); testTX.begin(); this.authenticationComponent.setSystemUserAsCurrentUser(); - - - - } public void testSetup() throws Exception