From d111a502523faa568d79d21871549ab3af4fd5df Mon Sep 17 00:00:00 2001 From: Elia Date: Tue, 23 Aug 2022 17:58:05 +0200 Subject: [PATCH 1/4] [MNT-23072] fix index last transaction error in dbid_range sharding added integration test --- .../solr/tracker/MetadataTracker.java | 12 ++-- ...ributedDbidRangeAlfrescoSolrTrackerIT.java | 67 ++++++++++++++++--- .../solr/client/SOLRAPIQueueClient.java | 20 ++++++ 3 files changed, 82 insertions(+), 17 deletions(-) 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 556e58a03..265bc290c 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 @@ -94,10 +94,6 @@ public class MetadataTracker extends ActivatableTracker private final ConcurrentLinkedQueue nodesToIndex = new ConcurrentLinkedQueue<>(); private final ConcurrentLinkedQueue nodesToPurge = new ConcurrentLinkedQueue<>(); private final ConcurrentLinkedQueue queriesToReindex = new ConcurrentLinkedQueue<>(); - - private final boolean isRunningInProduction = - !Boolean.parseBoolean(System.getProperty("alfresco.test", "false")); - private ForkJoinPool forkJoinPool; // Share run and write locks across all MetadataTracker threads @@ -193,7 +189,7 @@ public class MetadataTracker extends ActivatableTracker // In order to apply performance optimizations, checking the availability of Repo Web Scripts is required. // As these services are available from ACS 6.2 - if (checkRepoServicesAvailability && isRunningInProduction) + if (checkRepoServicesAvailability) { // Try invoking getNextTxCommitTime service try @@ -870,7 +866,7 @@ public class MetadataTracker extends ActivatableTracker latestTransaction.setCommitTimeMs(transactions.getMaxTxnCommitTime()); latestTransaction.setId(transactions.getMaxTxnId()); - if (!isTransactionIndexed(latestTransaction)) + if (isTransactionToBeIndexed(latestTransaction)) { transactions = new Transactions(Collections.singletonList(latestTransaction), transactions.getMaxTxnCommitTime(), transactions.getMaxTxnId()); @@ -887,7 +883,7 @@ public class MetadataTracker extends ActivatableTracker return transactions; } - private boolean isTransactionIndexed(Transaction transaction) + private boolean isTransactionToBeIndexed(Transaction transaction) { try { @@ -997,7 +993,7 @@ public class MetadataTracker extends ActivatableTracker final AtomicInteger counterTransaction = new AtomicInteger(); Collection> txBatches = transactions.getTransactions().stream() .peek(txnsFound::add) - .filter(this::isTransactionIndexed) + .filter(this::isTransactionToBeIndexed) .collect(Collectors.groupingBy(transaction -> counterTransaction.getAndAdd( (int) (transaction.getDeletes() + transaction.getUpdates())) / transactionDocsBatchSize)) .values(); diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java index fc5d52c90..d825370b0 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java @@ -37,6 +37,7 @@ import org.alfresco.solr.client.Transaction; import org.apache.lucene.index.Term; import org.apache.lucene.search.TermQuery; import org.apache.solr.SolrTestCaseJ4; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -47,6 +48,7 @@ import java.util.List; import java.util.Properties; import static org.alfresco.repo.search.adaptor.QueryConstants.FIELD_DOC_TYPE; +import static org.alfresco.solr.AlfrescoSolrUtils.MAX_WAIT_TIME; import static org.alfresco.solr.AlfrescoSolrUtils.getAcl; import static org.alfresco.solr.AlfrescoSolrUtils.getAclChangeSet; import static org.alfresco.solr.AlfrescoSolrUtils.getAclReaders; @@ -65,7 +67,7 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD @BeforeClass public static void initData() throws Throwable { - initSolrServers(2, getSimpleClassName(), getShardMethod()); + initSolrServers(3, getSimpleClassName(), getShardMethod()); } @AfterClass @@ -73,13 +75,14 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD { dismissSolrServers(); } - - @Test - public void testDbIdRange() throws Exception - { - putHandleDefaults(); - int numAcls = 250; + @After + public void deleteDataFromIndex() throws Exception { + deleteByQueryAllClients("*:*"); + waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), 0, MAX_WAIT_TIME); + } + + private List createAcls(int numAcls){ AclChangeSet bulkAclChangeSet = getAclChangeSet(numAcls); List bulkAcls = new ArrayList<>(); @@ -99,6 +102,17 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD bulkAcls, bulkAclReaders); + return bulkAcls; + } + + @Test + public void testDbIdRange() throws Exception + { + putHandleDefaults(); + + int numAcls = 250; + var bulkAcls = createAcls(numAcls); + int numNodes = 150; List nodes = new ArrayList<>(); List nodeMetaDatas = new ArrayList<>(); @@ -108,14 +122,14 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD for(int i=0; i getTxIntervalCommitTime(String coreName, Long fromNodeId, Long toNodeId) + { + List transactions = TRANSACTION_QUEUE.stream() + .filter(txn -> NODE_MAP.get(txn.getId()) + .stream() + .anyMatch(node -> node.getId() >= fromNodeId && node.getId() <= toNodeId)) + .collect(Collectors.toList()); + + if (transactions.size() > 0){ + return new Pair<>(transactions.get(0).getCommitTimeMs(), transactions.get(transactions.size() - 1).getCommitTimeMs()); + } else { + return new Pair<>(-1l, -1l); + } + } public Transactions getTransactions(Long fromCommitTime, Long minTxnId, Long toCommitTime, Long maxTxnId, int maxResults) throws IOException, JSONException { From e4bb483b29de2f6dbacf607f51ed16ce98fcdbb8 Mon Sep 17 00:00:00 2001 From: Elia Date: Wed, 24 Aug 2022 10:51:57 +0200 Subject: [PATCH 2/4] [MNT-23072] fix intermittend test failure --- ...ributedDbidRangeAlfrescoSolrTrackerIT.java | 66 +++++++++---------- .../solr/client/SOLRAPIQueueClient.java | 17 +++-- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java index d825370b0..cb824ed75 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java @@ -2,7 +2,7 @@ * #%L * Alfresco Search Services * %% - * Copyright (C) 2005 - 2020 Alfresco Software Limited + * Copyright (C) 2005 - 2022 Alfresco Software Limited * %% * This file is part of the Alfresco software. * If the software was purchased under a paid Alfresco license, the terms of @@ -23,7 +23,6 @@ * along with Alfresco. If not, see . * #L% */ - package org.alfresco.solr.tracker; import org.alfresco.solr.AbstractAlfrescoDistributedIT; @@ -33,6 +32,7 @@ import org.alfresco.solr.client.AclChangeSet; import org.alfresco.solr.client.AclReaders; import org.alfresco.solr.client.Node; import org.alfresco.solr.client.NodeMetaData; +import org.alfresco.solr.client.SOLRAPIQueueClient; import org.alfresco.solr.client.Transaction; import org.apache.lucene.index.Term; import org.apache.lucene.search.TermQuery; @@ -56,7 +56,6 @@ import static org.alfresco.solr.AlfrescoSolrUtils.getNode; import static org.alfresco.solr.AlfrescoSolrUtils.getNodeMetaData; import static org.alfresco.solr.AlfrescoSolrUtils.getTransaction; import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet; -import static org.alfresco.solr.AlfrescoSolrUtils.list; /** * @author Joel @@ -77,18 +76,23 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD } @After - public void deleteDataFromIndex() throws Exception { + public void deleteDataFromIndex() throws Exception + { + SOLRAPIQueueClient.TRANSACTION_QUEUE.clear(); + SOLRAPIQueueClient.NODE_MAP.clear(); deleteByQueryAllClients("*:*"); waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), 0, MAX_WAIT_TIME); } - private List createAcls(int numAcls){ + private List createAcls(int numAcls) + { AclChangeSet bulkAclChangeSet = getAclChangeSet(numAcls); List bulkAcls = new ArrayList<>(); List bulkAclReaders = new ArrayList<>(); - for(int i=0; i getTxIntervalCommitTime(String coreName, Long fromNodeId, Long toNodeId) { - List transactions = TRANSACTION_QUEUE.stream() + List transactionCommitTimestamps = TRANSACTION_QUEUE.stream() .filter(txn -> NODE_MAP.get(txn.getId()) .stream() .anyMatch(node -> node.getId() >= fromNodeId && node.getId() <= toNodeId)) + .map(tx -> tx.getCommitTimeMs()) + .sorted() .collect(Collectors.toList()); - if (transactions.size() > 0){ - return new Pair<>(transactions.get(0).getCommitTimeMs(), transactions.get(transactions.size() - 1).getCommitTimeMs()); - } else { + if (transactionCommitTimestamps.size() > 0) + { + return new Pair<>( transactionCommitTimestamps.get(0), transactionCommitTimestamps.get(transactionCommitTimestamps.size() - 1)); + } + else + { return new Pair<>(-1l, -1l); } } From 56504b62b9bd47e409375aa6e21cb50e34895534 Mon Sep 17 00:00:00 2001 From: Andrea Gazzarini Date: Tue, 30 Aug 2022 13:42:59 +0200 Subject: [PATCH 3/4] [MNT-23072] org.alfresco.solr.tracker.MetadataTracker#isTransactionToBeIndexed Unit tests --- .../solr/tracker/MetadataTracker.java | 2 +- ...ributedDbidRangeAlfrescoSolrTrackerIT.java | 20 +-- .../solr/tracker/MetadataTrackerTest.java | 114 +++++++++++++++--- 3 files changed, 107 insertions(+), 29 deletions(-) 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 265bc290c..ebfa390a9 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 @@ -883,7 +883,7 @@ public class MetadataTracker extends ActivatableTracker return transactions; } - private boolean isTransactionToBeIndexed(Transaction transaction) + boolean isTransactionToBeIndexed(Transaction transaction) { try { diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java index cb824ed75..5484bb8b4 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java @@ -145,16 +145,17 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD public void testIndexLastTransaction() throws Exception { var acls = createAcls(1); - int txId = 0; + int latestIndexedTransactionId = 0; // index 50 trx for each shard - for (int k = 0; k < 3; k++) + // Test configuration uses the following ranges: 0-100, 100-200, 200-300 (300-400 in case of a fourth shard which is not the case here) + for (int shardIndex = 0; shardIndex < 3; shardIndex++) { - for (int i = 0; i< 50; i++) + for (int nodeNumber = 0; nodeNumber< 50; nodeNumber++) { Transaction trx = getTransaction(0, 1); - trx.setId(txId++); - var node = getNode(k*100 + 10 + i, trx, acls.get(0), Node.SolrApiNodeStatus.UPDATED); + trx.setId(latestIndexedTransactionId++); + var node = getNode(shardIndex*100 + 10 + nodeNumber, trx, acls.get(0), Node.SolrApiNodeStatus.UPDATED); indexTransaction(trx, List.of(node), List.of(getNodeMetaData(node, trx, acls.get(0), "mike", null, false))); } } @@ -167,9 +168,12 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD assertShardCount(2, new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), 50); // check the last transaction has been indexed in all the shards - assertShardCount(0, new TermQuery(new Term("S_TXID", "149")), 1); - assertShardCount(1, new TermQuery(new Term("S_TXID", "149")), 1); - assertShardCount(2, new TermQuery(new Term("S_TXID", "149")), 1); + + var latestIndexedTransactionIdAsString = String.valueOf(latestIndexedTransactionId); + + assertShardCount(0, new TermQuery(new Term("S_TXID", latestIndexedTransactionIdAsString)), 1); + assertShardCount(1, new TermQuery(new Term("S_TXID", latestIndexedTransactionIdAsString)), 1); + assertShardCount(2, new TermQuery(new Term("S_TXID", latestIndexedTransactionIdAsString)), 1); } protected static Properties getShardMethod() diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/MetadataTrackerTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/MetadataTrackerTest.java index 388e3203e..7466a2576 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/MetadataTrackerTest.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/MetadataTrackerTest.java @@ -26,9 +26,6 @@ package org.alfresco.solr.tracker; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -36,7 +33,6 @@ import java.util.Properties; import org.alfresco.httpclient.AuthenticationException; import org.alfresco.repo.index.shard.ShardState; -import org.alfresco.solr.AlfrescoCoreAdminHandler; import org.alfresco.solr.InformationServer; import org.alfresco.solr.NodeReport; import org.alfresco.solr.TrackerState; @@ -56,6 +52,23 @@ import org.mockito.Mock; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + @RunWith(MockitoJUnitRunner.class) public class MetadataTrackerTest { @@ -75,6 +88,9 @@ public class MetadataTrackerTest @Mock private TrackerStats trackerStats; + @Mock + private TrackerState trackerState; + @Before public void setUp() { @@ -84,12 +100,9 @@ public class MetadataTrackerTest this.metadataTracker = spy(new MetadataTracker(props, repositoryClient, coreName, srv)); ModelTracker modelTracker = mock(ModelTracker.class); - when(modelTracker.hasModels()).thenReturn(true); - AlfrescoCoreAdminHandler adminHandler = mock(AlfrescoCoreAdminHandler.class); TrackerRegistry registry = new TrackerRegistry(); registry.setModelTracker(modelTracker); - when(adminHandler.getTrackerRegistry()).thenReturn(registry); - when(srv.getAdminHandler()).thenReturn(adminHandler); + metadataTracker.state = trackerState; } @Test @@ -188,6 +201,79 @@ public class MetadataTrackerTest assertEquals(TX_ID, nodeReport.getDbTx()); } + @Test + public void incomingCommitTimeIsLesserThanLastIndexedTxCommitTime_transactionShouldBeMarkedAsIndexed() throws Exception { + var incomingTransactionCommitTime = 10L; + var lastIndexedTransactionCommitTime = incomingTransactionCommitTime + 1; + + var incomingTransaction = new Transaction(); + incomingTransaction.setId(1); + incomingTransaction.setCommitTimeMs(incomingTransactionCommitTime); + + when(srv.txnInIndex(incomingTransaction.getId(), true)).thenReturn(true); + when(trackerState.getLastIndexedTxCommitTime()).thenReturn(lastIndexedTransactionCommitTime); + + assertFalse(metadataTracker.isTransactionToBeIndexed(incomingTransaction)); + } + + @Test + public void incomingCommitTimeIsLesserThanLastIndexedTxCommitTimeButTheTransactionIsNotIndexed_transactionShouldBeMarkedAsToBeIndexed() throws Exception { + var incomingTransactionCommitTime = 10L; + var lastIndexedTransactionCommitTime = incomingTransactionCommitTime + 1; + + var incomingTransaction = new Transaction(); + incomingTransaction.setId(1); + incomingTransaction.setCommitTimeMs(incomingTransactionCommitTime); + + when(srv.txnInIndex(incomingTransaction.getId(), true)).thenReturn(false); + when(trackerState.getLastIndexedTxCommitTime()).thenReturn(lastIndexedTransactionCommitTime); + + assertTrue(metadataTracker.isTransactionToBeIndexed(incomingTransaction)); + } + + @Test + public void incomingCommitTimeIsGreaterThanLastIndexedTxCommitTime_transactionShouldBeMarkedAsToBeIndexed() { + var lastIndexedTransactionCommitTime = 10L; + var incomingTransactionCommitTime = lastIndexedTransactionCommitTime + 1; + + var incomingTransaction = new Transaction(); + incomingTransaction.setId(1); + incomingTransaction.setCommitTimeMs(incomingTransactionCommitTime); + + when(trackerState.getLastIndexedTxCommitTime()).thenReturn(lastIndexedTransactionCommitTime); + + assertTrue(metadataTracker.isTransactionToBeIndexed(incomingTransaction)); + } + + @Test + public void incomingCommitTimeIsGreaterThanLastIndexedTxCommitTimeButTheTransactionIsAlreadyIndexed_transactionShouldBeMarkedAsToBeIndexed() { + var lastIndexedTransactionCommitTime = 10L; + var incomingTransactionCommitTime = lastIndexedTransactionCommitTime + 1; + + var incomingTransaction = new Transaction(); + incomingTransaction.setId(1); + incomingTransaction.setCommitTimeMs(incomingTransactionCommitTime); + + when(trackerState.getLastIndexedTxCommitTime()).thenReturn(lastIndexedTransactionCommitTime); + + assertTrue(metadataTracker.isTransactionToBeIndexed(incomingTransaction)); + } + + @Test + public void anIOExceptionIsRaised_transactionShouldBeMarkedAsToBeIndexed() throws Exception { + var incomingTransactionCommitTime = 10L; + var lastIndexedTransactionCommitTime = incomingTransactionCommitTime + 1; + + var incomingTransaction = new Transaction(); + incomingTransaction.setId(1); + incomingTransaction.setCommitTimeMs(incomingTransactionCommitTime); + + when(srv.txnInIndex(incomingTransaction.getId(), true)).thenThrow(new IOException()); + when(trackerState.getLastIndexedTxCommitTime()).thenReturn(lastIndexedTransactionCommitTime); + + assertTrue(metadataTracker.isTransactionToBeIndexed(incomingTransaction)); + } + private Node getNode() { Node node = new Node(); @@ -195,16 +281,4 @@ public class MetadataTrackerTest node.setTxnId(TX_ID); return node; } - - @Test - @Ignore("Superseded by AlfrescoSolrTrackerTest") - public void testGetFullNodesForDbTransaction() throws AuthenticationException, IOException, JSONException - { - List nodes = getNodes(); - when(repositoryClient.getNodes(any(GetNodesParameters.class), anyInt())).thenReturn(nodes); - - List nodes4Tx = this.metadataTracker.getFullNodesForDbTransaction(TX_ID); - - assertSame(nodes4Tx, nodes); - } } From 0f2617f0cf9fc531d752a23b288e8729c4fe0488 Mon Sep 17 00:00:00 2001 From: Andrea Gazzarini Date: Tue, 30 Aug 2022 15:53:34 +0200 Subject: [PATCH 4/4] [MNT-23072] org.alfresco.solr.tracker.MetadataTracker#isTransactionToBeIndexed Unit test Fix --- .../solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java index 5484bb8b4..d734399e1 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDbidRangeAlfrescoSolrTrackerIT.java @@ -169,7 +169,7 @@ public class DistributedDbidRangeAlfrescoSolrTrackerIT extends AbstractAlfrescoD // check the last transaction has been indexed in all the shards - var latestIndexedTransactionIdAsString = String.valueOf(latestIndexedTransactionId); + var latestIndexedTransactionIdAsString = String.valueOf(latestIndexedTransactionId - 1); assertShardCount(0, new TermQuery(new Term("S_TXID", latestIndexedTransactionIdAsString)), 1); assertShardCount(1, new TermQuery(new Term("S_TXID", latestIndexedTransactionIdAsString)), 1);