ACS-1907 TMDQ against MySql throws SQLException in certain situations (#679)

* Added alternative approach for DBQueryEngine to page through the entire query using "LIMIT" requests instead of result set streaming
* Use the alternative approach for MySql

.travis.yml

@@ -145,7 +145,7 @@ jobs:
       script: travis_wait 20 mvn -B test -pl repository -Dtest=AllDBTestsTestSuite -Ddb.name=alfresco -Ddb.url=jdbc:mariadb://localhost:3307/alfresco?useUnicode=yes\&characterEncoding=UTF-8 -Ddb.username=alfresco -Ddb.password=alfresco -Ddb.driver=org.mariadb.jdbc.Driver
 
     - name: "Repository - MariaDB 10.6 tests"
-      if: (branch =~ /(release\/.*$|master)/ AND commit_message !~ /\[skip db\]/ AND type != pull_request) OR commit_message =~ /\[db\]/
+      if: (branch =~ /(release\/.*$|master)/ AND commit_message !~ /\[skip db\]/ AND type != pull_request) OR commit_message =~ /\[db\]/ OR commit_message =~ /\[latest db\]/
       before_script:
         - docker run -d -p 3307:3306 --name mariadb -e MYSQL_ROOT_PASSWORD=alfresco -e MYSQL_USER=alfresco -e MYSQL_DATABASE=alfresco -e MYSQL_PASSWORD=alfresco mariadb:10.6 --transaction-isolation=READ-COMMITTED --max-connections=300 --character-set-server=utf8mb4 --collation-server=utf8mb4_unicode_ci
         - docker run -d -p 61616:61616 -p 5672:5672 alfresco/alfresco-activemq:5.16.1
@@ -159,7 +159,7 @@ jobs:
       script: travis_wait 20 mvn -B test -pl repository -Dtest=AllDBTestsTestSuite -Ddb.driver=com.mysql.jdbc.Driver -Ddb.name=alfresco -Ddb.url=jdbc:mysql://localhost:3307/alfresco -Ddb.username=alfresco -Ddb.password=alfresco
 
     - name: "Repository - MySQL 8 tests"
-      if: (branch =~ /(release\/.*$|master)/ AND commit_message !~ /\[skip db\]/ AND type != pull_request) OR commit_message =~ /\[db\]/
+      if: (branch =~ /(release\/.*$|master)/ AND commit_message !~ /\[skip db\]/ AND type != pull_request) OR commit_message =~ /\[db\]/ OR commit_message =~ /\[latest db\]/
       before_script:
         - docker run -d -p 3307:3306 -e MYSQL_ROOT_PASSWORD=alfresco -e MYSQL_USER=alfresco -e MYSQL_DATABASE=alfresco -e MYSQL_PASSWORD=alfresco  mysql:8 --transaction-isolation='READ-COMMITTED'
         - docker run -d -p 61616:61616 -p 5672:5672 alfresco/alfresco-activemq:5.16.1
@@ -210,7 +210,7 @@ jobs:
 
     - name: "Repository - PostgreSQL 13.3 tests"
       # We only run DB tests on the latest version of PostgreSQL on feature branches
-      if: commit_message !~ /\[skip db\]/
+      if: commit_message !~ /\[skip db\]/ OR commit_message =~ /\[latest db\]/
       before_script:
         - docker run -d -p 5433:5432 -e POSTGRES_PASSWORD=alfresco -e POSTGRES_USER=alfresco -e POSTGRES_DB=alfresco postgres:13.3 postgres -c 'max_connections=300'
         - docker run -d -p 61616:61616 -p 5672:5672 alfresco/alfresco-activemq:5.16.1

data-model/src/main/java/org/alfresco/repo/search/impl/querymodel/QueryEngine.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Data model classes
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 

repository/src/main/java/org/alfresco/repo/search/impl/querymodel/impl/db/DBQuery.java

@@ -1,28 +1,28 @@
-/*
- * #%L
- * Alfresco Repository
- * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
- * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
- * 
- * 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 <http://www.gnu.org/licenses/>.
- * #L%
- */
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software. 
+ * If the software was purchased under a paid Alfresco license, the terms of 
+ * the paid license agreement will prevail.  Otherwise, the software is 
+ * provided under the following open source license terms:
+ * 
+ * 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 <http://www.gnu.org/licenses/>.
+ * #L%
+ */
 package org.alfresco.repo.search.impl.querymodel.impl.db;
 
 import java.util.ArrayList;
@@ -74,6 +74,10 @@ public class DBQuery extends BaseQuery implements DBQueryBuilderComponent
     
     Set<String> selectorGroup;
 
+    private int limit = 0;
+
+    private int offset = 0;
+
     /**
      * @param source Source
      * @param constraint Constraint
@@ -133,6 +137,22 @@ public class DBQuery extends BaseQuery implements DBQueryBuilderComponent
         this.sinceTxId = sinceTxId;
     }
 
+    public int getLimit() {
+        return limit;
+    }
+
+    public void setLimit(int limit) {
+        this.limit = limit;
+    }
+
+    public int getOffset() {
+        return offset;
+    }
+
+    public void setOffset(int offset) {
+        this.offset = offset;
+    }
+
     public List<DBQueryBuilderJoinCommand> getJoins()
     {
         HashMap<QName, DBQueryBuilderJoinCommand> singleJoins = new HashMap<QName, DBQueryBuilderJoinCommand>();

repository/src/main/java/org/alfresco/repo/search/impl/querymodel/impl/db/DBQueryEngine.java

@@ -76,8 +76,10 @@ import org.alfresco.service.namespace.QName;
 import org.alfresco.util.Pair;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.ibatis.executor.result.DefaultResultContext;
 import org.apache.ibatis.session.ResultContext;
 import org.apache.ibatis.session.ResultHandler;
+import org.apache.ibatis.session.RowBounds;
 import org.mybatis.spring.SqlSessionTemplate;
 
 /**
@@ -89,7 +91,11 @@ public class DBQueryEngine implements QueryEngine
     protected static final Log logger = LogFactory.getLog(DBQueryEngine.class);
     
     protected static final String SELECT_BY_DYNAMIC_QUERY = "alfresco.metadata.query.select_byDynamicQuery";
-    
+
+    private static final int DEFAULT_MIN_PAGING_BATCH_SIZE = 2500;
+
+    private static final int DEFAULT_MAX_PAGING_BATCH_SIZE = 10000;
+
     protected SqlSessionTemplate template;
 
     protected QNameDAO qnameDAO;
@@ -114,6 +120,12 @@ public class DBQueryEngine implements QueryEngine
 
     private boolean maxPermissionCheckEnabled;
 
+    private boolean usePagingQuery = false;
+
+    private int minPagingBatchSize = DEFAULT_MIN_PAGING_BATCH_SIZE;
+
+    private int maxPagingBatchSize = DEFAULT_MAX_PAGING_BATCH_SIZE;
+
     protected EntityLookupCache<Long, Node, NodeRef> nodesCache;
 
     private List<Pair<Long, StoreRef>> stores;
@@ -149,7 +161,31 @@ public class DBQueryEngine implements QueryEngine
     {
         this.permissionService = permissionService;
     }
-    
+
+    public boolean isUsePagingQuery() {
+        return usePagingQuery;
+    }
+
+    public void setUsePagingQuery(boolean usePagingQuery) {
+        this.usePagingQuery = usePagingQuery;
+    }
+
+    public int getMinPagingBatchSize() {
+        return minPagingBatchSize;
+    }
+
+    public void setMinPagingBatchSize(int minPagingBatchSize) {
+        this.minPagingBatchSize = minPagingBatchSize;
+    }
+
+    public int getMaxPagingBatchSize() {
+        return maxPagingBatchSize;
+    }
+
+    public void setMaxPagingBatchSize(int maxPagingBatchSize) {
+        this.maxPagingBatchSize = maxPagingBatchSize;
+    }
+
     public void setMetadataIndexCheck2(OptionalPatchApplicationCheckBootstrapBean metadataIndexCheck2)
     {
         this.metadataIndexCheck2 = metadataIndexCheck2;
@@ -331,7 +367,7 @@ public class DBQueryEngine implements QueryEngine
         int requiredNodes = computeRequiredNodesCount(options);
         
         logger.debug("- query sent to the database");
-        template.select(pickQueryTemplate(options, dbQuery), dbQuery, new ResultHandler<Node>()
+        performTmdqSelect(pickQueryTemplate(options, dbQuery), dbQuery, requiredNodes, new ResultHandler<Node>()
         {
             @Override
             public void handleResult(ResultContext<? extends Node> context)
@@ -399,6 +435,54 @@ public class DBQueryEngine implements QueryEngine
         return frs;
     }
 
+    private void performTmdqSelect(String statement, DBQuery dbQuery, int requiredNodes, ResultHandler<Node> handler)
+    {
+        if (usePagingQuery)
+        {
+            performTmdqSelectPaging(statement, dbQuery, requiredNodes, handler);
+        }
+        else
+        {
+            performTmdqSelectStreaming(statement, dbQuery, handler);
+        }
+    }
+
+    private void performTmdqSelectStreaming(String statement, DBQuery dbQuery, ResultHandler<Node> handler)
+    {
+        template.select(statement, dbQuery, handler);
+    }
+
+    private void performTmdqSelectPaging(String statement, DBQuery dbQuery, int requiredNodes, ResultHandler<Node> handler)
+    {
+        int batchStart = 0;
+        int batchSize = requiredNodes * 2;
+        batchSize = Math.min(Math.max(batchSize, minPagingBatchSize), maxPagingBatchSize);
+        DefaultResultContext<Node> resultCtx = new DefaultResultContext<>();
+        while (!resultCtx.isStopped())
+        {
+            dbQuery.setOffset(batchStart);
+            dbQuery.setLimit(batchSize);
+            List<Node> batch = template.selectList(statement, dbQuery);
+            for (Node node : batch)
+            {
+                resultCtx.nextResultObject(node);
+                handler.handleResult(resultCtx);
+                if (resultCtx.isStopped())
+                {
+                    break;
+                }
+            }
+            if (batch.size() < batchSize)
+            {
+                resultCtx.stop();
+            }
+            else
+            {
+                batchStart += batchSize;
+            }
+        }
+    }
+
     private DBResultSet createResultSet(QueryOptions options, List<Node> nodes, int numberFound)
     {
         DBResultSet dbResultSet = new DBResultSet(options.getAsSearchParmeters(), nodes, nodeDAO, nodeService, tenantService, Integer.MAX_VALUE);

repository/src/main/resources/alfresco/ibatis/org.alfresco.repo.domain.dialect.Dialect/metadata-query-common-SqlMap.xml

@@ -177,7 +177,8 @@
                  </choose>
             </foreach>
           </if>
-        </if>    
-    </sql>
+        </if>
+        <if test="limit != 0">limit #{offset}, #{limit}</if>
+   </sql>
 
 </mapper>

repository/src/main/resources/alfresco/subsystems/Search/common-search-context.xml

@@ -105,8 +105,35 @@
             <ref bean="metadataQueryIndexesCheck2" />
         </property>
     </bean>
-    
-    <bean id="search.dbQueryEngineImpl" class="org.alfresco.repo.search.impl.querymodel.impl.db.DBQueryEngine" >
+
+    <bean id="search.dbQueryEngineImpl" class="org.alfresco.util.bean.HierarchicalBeanLoader">
+        <property name="targetBeanName">
+            <value>search.dbQueryEngineImpl.#bean.dialect#</value>
+        </property>
+        <property name="targetClass">
+            <value>org.alfresco.repo.search.impl.querymodel.QueryEngine</value>
+        </property>
+        <property name="dialectBaseClass">
+            <value>org.alfresco.repo.domain.dialect.Dialect</value>
+        </property>
+        <property name="dialectClass">
+            <bean class="org.springframework.beans.factory.config.PropertyPathFactoryBean">
+                <property name="targetBeanName" value="dialect" />
+                <property name="propertyPath" value="class.name" />
+            </bean>
+        </property>
+    </bean>
+
+    <bean id="search.dbQueryEngineImpl.org.alfresco.repo.domain.dialect.Dialect"
+          parent="search.baseDbQueryEngineImpl">
+        <property name="usePagingQuery" value="false"/>
+    </bean>
+    <bean id="search.dbQueryEngineImpl.org.alfresco.repo.domain.dialect.MySQLInnoDBDialect"
+          parent="search.baseDbQueryEngineImpl">
+        <property name="usePagingQuery" value="true"/>
+    </bean>
+
+    <bean id="search.baseDbQueryEngineImpl" class="org.alfresco.repo.search.impl.querymodel.impl.db.DBQueryEngine" abstract="true">
         <property name="permissionService" ref="permissionService"/>
         <property name="dictionaryService" ref="dictionaryService" />
         <property name="namespaceService" ref="namespaceService" />

repository/src/test/java/org/alfresco/AllDBTestsTestSuite.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2017 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software.
  * If the software was purchased under a paid Alfresco license, the terms of
@@ -80,7 +80,7 @@ import org.junit.runners.Suite;
     // ACS-1907
     org.alfresco.repo.search.impl.querymodel.impl.db.ACS1907Test.class,
 
-    // REPO-2963 : Tests causing a cascade of failures in AllDBTestsTestSuite on PostgreSQL/MySQL	
+    // REPO-2963 : Tests causing a cascade of failures in AllDBTestsTestSuite on PostgreSQL/MySQL
     // Moved at the bottom of the suite because DbNodeServiceImplTest.testNodeCleanupRegistry() takes a long time on a clean DB.
     org.alfresco.repo.node.db.DbNodeServiceImplTest.class,
 

repository/src/test/java/org/alfresco/repo/search/impl/querymodel/impl/db/ACS1907Test.java

@@ -28,6 +28,7 @@ package org.alfresco.repo.search.impl.querymodel.impl.db;
 import junit.framework.TestCase;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.cache.TransactionalCache;
+import org.alfresco.repo.management.subsystems.SwitchableApplicationContextFactory;
 import org.alfresco.repo.security.authentication.AuthenticationComponent;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.MutableAuthenticationDao;
@@ -36,21 +37,23 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper;
 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.search.ResultSet;
-import org.alfresco.service.cmr.search.ResultSetRow;
-import org.alfresco.service.cmr.search.SearchParameters;
-import org.alfresco.service.cmr.search.SearchService;
+import org.alfresco.service.cmr.search.*;
 import org.alfresco.service.cmr.security.MutableAuthenticationService;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
+import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.testing.category.DBTests;
+import org.junit.experimental.categories.Category;
 import org.springframework.context.ApplicationContext;
 
 import java.io.Serializable;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 
+@Category({OwnJVMTestsCategory.class, DBTests.class})
 public class ACS1907Test extends TestCase
 {
 
@@ -64,6 +67,7 @@ public class ACS1907Test extends TestCase
     private PermissionService pubPermissionService;
     private TransactionService transactionService;
     private RetryingTransactionHelper txnHelper;
+    private DBQueryEngine queryEngine;
 
     private TransactionalCache<Serializable, AccessControlList> aclCache;
     private TransactionalCache<Serializable, Object> aclEntityCache;
@@ -101,7 +105,16 @@ public class ACS1907Test extends TestCase
         aclCache = (TransactionalCache) ctx.getBean("aclCache");
         aclEntityCache = (TransactionalCache) ctx.getBean("aclEntityCache");
         permissionEntityCache = (TransactionalCache) ctx.getBean("permissionEntityCache");
-        txnHelper = transactionService.getRetryingTransactionHelper();
+        SwitchableApplicationContextFactory searchContextFactory = (SwitchableApplicationContextFactory) ctx.getBean("Search");
+        ApplicationContext searchCtx = searchContextFactory.getApplicationContext();
+        queryEngine = (DBQueryEngine) searchCtx.getBean("search.dbQueryEngineImpl");
+        txnHelper = new RetryingTransactionHelper();
+        txnHelper.setTransactionService(transactionService);
+        txnHelper.setReadOnly(false);
+        txnHelper.setMaxRetries(1);
+        txnHelper.setMinRetryWaitMs(1);
+        txnHelper.setMaxRetryWaitMs(10);
+        txnHelper.setRetryWaitIncrementMs(1);
     }
 
     private void setupTestUser(String userName)
@@ -127,7 +140,7 @@ public class ACS1907Test extends TestCase
 
     private void setupTestContent()
     {
-        for(int f = 0; f < 100; f++)
+        for(int f = 0; f < 5; f++)
         {
             final int ff = f;
             txnHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Object>() {
@@ -142,7 +155,7 @@ public class ACS1907Test extends TestCase
                             ContentModel.TYPE_FOLDER,
                             testFolderProps
                     ).getChildRef();
-                    for(int c = 0; c < 1000; c++)
+                    for(int c = 0; c < 5; c++)
                     {
                         Map<QName, Serializable> testContentProps = new HashMap<>();
                         testContentProps.put(ContentModel.PROP_NAME, "content"+c);
@@ -179,12 +192,15 @@ public class ACS1907Test extends TestCase
                     public Object doWork() throws Exception {
                         SearchParameters sp = new SearchParameters();
                         sp.addStore(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE);
-                        sp.setLanguage(SearchService.LANGUAGE_LUCENE);
+                        sp.setLanguage(SearchService.LANGUAGE_FTS_ALFRESCO);
+                        sp.setQueryConsistency(QueryConsistency.TRANSACTIONAL);
                         sp.setQuery("TYPE:\"cm:content\"");
                         ResultSet rs = pubSearchService.query(sp);
+                        int cnt = 0;
                         for (ResultSetRow row : rs)
                         {
                             assertNotNull(row.getValue(ContentModel.PROP_NAME));
+                            cnt++;
                         }
                         return null;
                     }
@@ -194,4 +210,51 @@ public class ACS1907Test extends TestCase
         }, false, false);
     }
 
+    public void testPaging()
+    {
+        HashSet<NodeRef> resultPageSize2 = queryNodes(2);
+        HashSet<NodeRef> resultPageSize5 = queryNodes(5);
+        HashSet<NodeRef> resultPageSize10 = queryNodes(10);
+        HashSet<NodeRef> resultPageSizeAll = queryNodes(10000);
+        // all result sets must be equal, independent of page size used to retrieve them
+        assertTrue(resultPageSize2.size() >= 25);
+        assertTrue(resultPageSize5.size() >= 25);
+        assertTrue(resultPageSize10.size() >= 25);
+        assertTrue(resultPageSizeAll.size() >= 25);
+        assertTrue(resultPageSize2.containsAll(resultPageSize5));
+        assertTrue(resultPageSize2.containsAll(resultPageSize10));
+        assertTrue(resultPageSize2.containsAll(resultPageSizeAll));
+        assertTrue(resultPageSize5.containsAll(resultPageSize2));
+        assertTrue(resultPageSize5.containsAll(resultPageSize10));
+        assertTrue(resultPageSize5.containsAll(resultPageSizeAll));
+        assertTrue(resultPageSize10.containsAll(resultPageSize2));
+        assertTrue(resultPageSize10.containsAll(resultPageSize5));
+        assertTrue(resultPageSize10.containsAll(resultPageSizeAll));
+        assertTrue(resultPageSizeAll.containsAll(resultPageSize2));
+        assertTrue(resultPageSizeAll.containsAll(resultPageSize5));
+        assertTrue(resultPageSizeAll.containsAll(resultPageSize10));
+        // reset
+        queryEngine.setMinPagingBatchSize(2500);
+        queryEngine.setMaxPagingBatchSize(10000);
+    }
+
+    HashSet<NodeRef> queryNodes(int pageSize)
+    {
+        queryEngine.setMinPagingBatchSize(pageSize);
+        queryEngine.setMaxPagingBatchSize(pageSize);
+        HashSet<NodeRef> result = new HashSet<>();
+        SearchParameters sp = new SearchParameters();
+        sp.addStore(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE);
+        sp.setLanguage(SearchService.LANGUAGE_FTS_ALFRESCO);
+        sp.setQueryConsistency(QueryConsistency.TRANSACTIONAL);
+        sp.setQuery("TYPE:\"cm:content\"");
+        ResultSet rs = pubSearchService.query(sp);
+        int cnt = 0;
+        for (ResultSetRow row : rs)
+        {
+            result.add(row.getNodeRef());
+        }
+        return result;
+    }
+
 }