From 48a93e6ce4bff40799de5e855a94c4ec0f52161d Mon Sep 17 00:00:00 2001 From: Joel Date: Tue, 3 Apr 2018 14:06:18 -0400 Subject: [PATCH] Squashed commit of the following: commit 9edb082eeb0f4610c4f6a27273585631c1bd69d9 Author: Joel Date: Tue Apr 3 13:20:02 2018 -0400 MNT-19180: AclTracker code clarification and remove commented debugging commit 3109b3a3f26557e2f03725514d63f74aee18e044 Author: Michael Date: Fri Mar 30 16:48:28 2018 +0100 MNT-19180: Fix intermittent failing test caused by the fix to tracker race condition. commit fd9a4e5c0ae2a1785b7e239244a5ef07d4e376f0 Author: Joel Date: Sun Mar 25 21:37:04 2018 -0400 MNT-19180: Add DistributedAlfrescoSolrTrackerRaceTest commit d4ba8594742691c483a104ce47f2d5220b61bc58 Author: Joel Date: Tue Mar 20 21:05:42 2018 -0400 MNT-19180: Fix race condition with TrackerState --- .../solr/tracker/AbstractTracker.java | 65 ++++++++-- .../org/alfresco/solr/tracker/AclTracker.java | 11 +- .../solr/tracker/MetadataTracker.java | 24 +--- .../solr/AbstractAlfrescoDistributedTest.java | 3 +- ...istributedAlfrescoSolrTrackerRaceTest.java | 122 ++++++++++++++++++ 5 files changed, 192 insertions(+), 33 deletions(-) create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedAlfrescoSolrTrackerRaceTest.java diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AbstractTracker.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AbstractTracker.java index b55f3702c..d55fc274c 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AbstractTracker.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AbstractTracker.java @@ -130,7 +130,42 @@ public abstract class AbstractTracker implements Tracker * @throws Throwable */ protected abstract void doTrack() throws Throwable; - + + + private boolean assertTrackerStateRemainsNull() { + + /* + * This assertion is added to accommodate DistributedAlfrescoSolrTrackerRaceTest. + * The sleep is needed to allow the test case to add a txn into the queue before + * the tracker makes its call to pull transactions from the test repo client. + */ + + try + { + Thread.sleep(5000); + } + catch(Exception e) + { + + } + + + /* + * This ensures that getTrackerState does not have the side effect of setting the + * state instance variable. This allows classes outside of the tracker framework + * to safely call getTrackerState without interfering with the trackers design. + */ + + getTrackerState(); + + + if(state == null) { + return true; + } else { + return false; + } + + } /** * Template method - subclasses must implement the {@link Tracker}-specific indexing * by implementing the abstract method {@link #doTrack()}. @@ -146,17 +181,26 @@ public abstract class AbstractTracker implements Tracker try { runLock.acquire(); + + if(state==null && Boolean.parseBoolean(System.getProperty("alfresco.test", "false"))) + { + assert(assertTrackerStateRemainsNull()); + } + log.info("... Running " + this.getClass().getSimpleName() + " for core [" + coreName + "]."); - if(state == null) + if(this.state == null) { - getTrackerState(); - state.setRunning(true); + /* + * Set the global state for the tracker here. + */ + this.state = getTrackerState(); + this.state.setRunning(true); } else { continueState(); - state.setRunning(true); + this.state.setRunning(true); } infoSrv.registerTrackerThread(); @@ -221,22 +265,21 @@ public abstract class AbstractTracker implements Tracker state.incrementTrackerCycles(); } - public void invalidateState() + public synchronized void invalidateState() { state = null; } @Override - public TrackerState getTrackerState() + public synchronized TrackerState getTrackerState() { - if(state != null) + if(this.state != null) { - return state; + return this.state; } else { - state = this.infoSrv.getTrackerInitialState(); - return state; + return this.infoSrv.getTrackerInitialState(); } } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AclTracker.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AclTracker.java index c2e782b6a..f21ef9085 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AclTracker.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/AclTracker.java @@ -642,16 +642,19 @@ public class AclTracker extends AbstractTracker try { getWriteLock().acquire(); - //System.out.println("############# AclTracker acquire lock ################"); - TrackerState state = getTrackerState(); + /* + * We acquire the tracker state again here and set it globally. This is because the + * tracker state could have been invalidated due to a rollback by the CommitTracker. + * In this case the state will revert to the last transaction state record in the index. + */ + + this.state = getTrackerState(); Long fromCommitTime = getChangeSetFromCommitTime(changeSetsFound, state.getLastGoodChangeSetCommitTimeInIndex()); aclChangeSets = getSomeAclChangeSets(changeSetsFound, fromCommitTime, TIME_STEP_1_HR_IN_MS, 2000, state.getTimeToStopIndexing()); - //System.out.println("############# Changesets ################:"+aclChangeSets.getAclChangeSets().size()); - setLastChangeSetIdAndCommitTimeInTrackerState(aclChangeSets, state); diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java index 2bf6812be..824d47c00 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java @@ -104,6 +104,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker @Override protected void doTrack() throws AuthenticationException, IOException, JSONException, EncoderException { + // MetadataTracker must wait until ModelTracker has run ModelTracker modelTracker = this.infoSrv.getAdminHandler().getTrackerRegistry().getModelTracker(); @@ -137,7 +138,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker private void trackRepository() throws IOException, AuthenticationException, JSONException, EncoderException { - //System.out.println("########################### MetadataTracker.trackRepository ##########"); checkShutdown(); if(!isMaster && isSlave) @@ -243,7 +243,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker Transaction firstTransaction = firstTransactions.getTransactions().get(0); long firstTxId = firstTransaction.getId(); long firstTransactionCommitTime = firstTransaction.getCommitTimeMs(); - //System.out.println("###### First txn:"+firstTxId+" : "+firstTransactionCommitTime); int setSize = this.infoSrv.getTxDocsSize(""+firstTxId, ""+firstTransactionCommitTime); if (setSize == 0) @@ -575,7 +574,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker protected Transactions getSomeTransactions(BoundedDeque txnsFound, Long fromCommitTime, long timeStep, int maxResults, long endTime) throws AuthenticationException, IOException, JSONException, EncoderException { - //System.out.println("###################### getSomeTransactions ####### "+fromCommitTime+":"+endTime); long actualTimeStep = timeStep; @@ -593,7 +591,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker } while (((transactions.getTransactions().size() == 0) && (startTime < endTime)) || ((transactions.getTransactions().size() > 0) && alreadyFoundTransactions(txnsFound, transactions))); - //System.out.println("############ done with transactions ######### " + startTime + ":" + endTime + ":" + transactions.getTransactions().size()); return transactions; } @@ -614,22 +611,20 @@ public class MetadataTracker extends AbstractTracker implements Tracker try { getWriteLock().acquire(); - //System.out.println("######## Metadata Tracket Acquiring Write Lock ########"); - /* - * Check to see if we are using the capped router, and if the cap has already been set. - */ + /* + * We acquire the tracker state again here and set it globally. This is because the + * tracker state could have been invalidated due to a rollback by the CommitTracker. + * In this case the state will revert to the last transaction state record in the index. + */ - TrackerState state = getTrackerState(); - - //System.out.println("######## Do Track Transactions ########:"+docCount); + this.state = getTrackerState(); Long fromCommitTime = getTxFromCommitTime(txnsFound, state.getLastGoodTxCommitTimeInIndex()); transactions = getSomeTransactions(txnsFound, fromCommitTime, TIME_STEP_1_HR_IN_MS, 2000, state.getTimeToStopIndexing()); - //System.out.println("######## get some! ########:"+transactions.getTransactions().size()); setLastTxCommitTimeAndTxIdInTrackerState(transactions, state); @@ -645,17 +640,13 @@ public class MetadataTracker extends AbstractTracker implements Tracker ArrayList txBatch = new ArrayList<>(); for (Transaction info : transactions.getTransactions()) { - //System.out.println("############## Transaction:"+ info.getId()); boolean isInIndex = (infoSrv.txnInIndex(info.getId(), true) && info.getCommitTimeMs() <= state.getLastIndexedTxCommitTime()); if (isInIndex) { - //System.out.println("################## Transaction Found In Index:"+info.getId()); txnsFound.add(info); } else { - //System.out.println("################## New Transaction:"+info.getId()); // Make sure we do not go ahead of where we started - we will check the holes here // correctly next time if (info.getCommitTimeMs() > state.getTimeToStopIndexing()) { - //System.out.println("################## We are upToDate:"+state.getTimeToStopIndexing()); upToDate = true; break; } @@ -719,7 +710,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker finally { getWriteLock().release(); - //System.out.println("######## Metadata Tracket Releasing Write Lock ########:"+getWriteLock().availablePermits()); } } while ((transactions.getTransactions().size() > 0) && (upToDate == false)); diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/AbstractAlfrescoDistributedTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AbstractAlfrescoDistributedTest.java index d279f3315..eaa8af5ea 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/AbstractAlfrescoDistributedTest.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AbstractAlfrescoDistributedTest.java @@ -1,5 +1,6 @@ package org.alfresco.solr; +import com.carrotsearch.ant.tasks.junit4.dependencies.com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering; import org.alfresco.solr.client.Node; import org.alfresco.solr.client.NodeMetaData; @@ -84,7 +85,7 @@ import static org.alfresco.solr.AlfrescoSolrUtils.createCoreUsingTemplate; * @since solr 1.5 * @author Michael Suzuki */ -@ThreadLeakLingering(linger = 2000) +@ThreadLeakLingering(linger = 5000) public abstract class AbstractAlfrescoDistributedTest extends SolrTestCaseJ4 { // TODO: this shouldn't be static. get the random when you need it to avoid diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedAlfrescoSolrTrackerRaceTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedAlfrescoSolrTrackerRaceTest.java new file mode 100644 index 000000000..12d9b1583 --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedAlfrescoSolrTrackerRaceTest.java @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco 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 Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.solr.tracker; + +import org.alfresco.solr.AbstractAlfrescoDistributedTest; +import org.alfresco.repo.search.adaptor.lucene.QueryConstants; +import org.alfresco.solr.client.*; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.LegacyNumericRangeQuery; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.util.LuceneTestCase; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.junit.Rule; +import org.junit.Test; + +import static org.alfresco.solr.AlfrescoSolrUtils.*; +import static org.alfresco.solr.AlfrescoSolrUtils.getAclReaders; +import static org.alfresco.solr.AlfrescoSolrUtils.list; + +import java.util.List; +import java.util.ArrayList; + +/** + * @author Joel + */ +@SolrTestCaseJ4.SuppressSSL +@LuceneTestCase.SuppressCodecs({"Appending","Lucene3x","Lucene40","Lucene41","Lucene42","Lucene43", "Lucene44", "Lucene45","Lucene46","Lucene47","Lucene48","Lucene49"}) +public class DistributedAlfrescoSolrTrackerRaceTest extends AbstractAlfrescoDistributedTest +{ + + @Rule + public JettyServerRule jetty = new JettyServerRule(2, this); + + @Test + public void testTracker() throws Exception + { + putHandleDefaults(); + + + AclChangeSet aclChangeSet = getAclChangeSet(1); + + Acl acl = getAcl(aclChangeSet); + Acl acl2 = getAcl(aclChangeSet); + + AclReaders aclReaders = getAclReaders(aclChangeSet, acl, list("joel"), list("phil"), null); + AclReaders aclReaders2 = getAclReaders(aclChangeSet, acl2, list("jim"), list("phil"), null); + + Transaction txn = getTransaction(0, 2); + long txnCommitTimeMs = txn.getCommitTimeMs(); + + //Subtract from the commit time to go beyond hole retention + long backdatedCommitTimeMs = txnCommitTimeMs-(4600000); + txn.setCommitTimeMs(backdatedCommitTimeMs); + + //Next create two nodes to update for the transaction + Node folderNode = getNode(txn, acl, Node.SolrApiNodeStatus.UPDATED); + Node fileNode = getNode(txn, acl, Node.SolrApiNodeStatus.UPDATED); + Node errorNode = getNode(txn, acl, Node.SolrApiNodeStatus.UPDATED); + + + //Next create the NodeMetaData for each node. TODO: Add more metadata + NodeMetaData folderMetaData = getNodeMetaData(folderNode, txn, acl, "mike", null, false); + NodeMetaData fileMetaData = getNodeMetaData(fileNode, txn, acl, "mike", ancestors(folderMetaData.getNodeRef()), false); + //The errorNodeMetaData will cause an exception. + NodeMetaData errorMetaData = getNodeMetaData(errorNode, txn, acl, "lisa", ancestors(folderMetaData.getNodeRef()), true); + + //Index the transaction, nodes, and nodeMetaDatas. + //Note that the content is automatically created by the test framework. + indexTransaction(txn, + list(errorNode, folderNode, fileNode), + list(errorMetaData, folderMetaData, fileMetaData)); + + + indexAclChangeSet(aclChangeSet, + list(acl, acl2), + list(aclReaders, aclReaders2)); + + System.out.println(backdatedCommitTimeMs + ":" + txnCommitTimeMs); + + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + builder.add(new BooleanClause(new TermQuery(new Term(QueryConstants.FIELD_SOLR4_ID, "TRACKER!STATE!ACLTX")), BooleanClause.Occur.MUST)); + builder.add(new BooleanClause(LegacyNumericRangeQuery.newLongRange(QueryConstants.FIELD_S_ACLTXID, aclChangeSet.getId(), aclChangeSet.getId() + 1, true, false), BooleanClause.Occur.MUST)); + BooleanQuery waitForQuery = builder.build(); + waitForDocCountAllCores(waitForQuery, 1, 80000); + + /* + * Query the index for the content + */ + //This acl should have one record in each core with DBID sharding + waitForDocCountAllCores(new TermQuery(new Term(QueryConstants.FIELD_READER, "jim")), 1, 80000); + waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), 2, 100000); + waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", Long.toString(fileNode.getId()))), 1, 80000); + + + //This will run the query on the control client and the cluster and compare the result. + query(getDefaultTestClient(), true, "{\"locales\":[\"en\"], \"templates\": [{\"name\":\"t1\", \"template\":\"%cm:content\"}]}", + params("q", "t1:world", "qt", "/afts", "shards.qt", "/afts", "start", "0", "rows", "6", "sort", "id asc")); + + } + + +} +