Merge pull request #4536 from xwiki/XWIKI-15166-2

XWIKI-15166: SOLR reindexation job cause serious perfomance issues on large database

Author: tmortagne

xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/job/AbstractDocumentIterator.java

@@ -21,8 +21,12 @@
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.xwiki.model.reference.EntityReference;
+import org.xwiki.model.reference.WikiReference;
 import org.xwiki.search.solr.internal.api.SolrConfiguration;
+import org.xwiki.text.XWikiToStringBuilder;
 
 /**
  * Base class for {@link DocumentIterator}s.
@@ -33,6 +37,96 @@
  */
 public abstract class AbstractDocumentIterator<T> implements DocumentIterator<T>
 {
+    /**
+     * A document related entry.
+     * 
+     * @version $Id$
+     * @since 17.8.0RC1
+     */
+    public static class DocumentIteratorEntry
+    {
+        private final WikiReference wiki;
+
+        private final long docId;
+
+        private final String version;
+
+        protected DocumentIteratorEntry(WikiReference wiki, long docId, String version)
+        {
+            this.wiki = wiki;
+            this.docId = docId;
+            this.version = version;
+        }
+
+        /**
+         * @return the reference
+         */
+        public WikiReference getWiki()
+        {
+            return wiki;
+        }
+
+        /**
+         * @return the docId
+         */
+        public long getDocId()
+        {
+            return docId;
+        }
+
+        /**
+         * @return the version
+         */
+        public String getVersion()
+        {
+            return version;
+        }
+
+        @Override
+        public int hashCode()
+        {
+            HashCodeBuilder builder = new HashCodeBuilder();
+
+            builder.append(getWiki());
+            builder.append(getDocId());
+            builder.append(getVersion());
+
+            return builder.build();
+        }
+
+        @Override
+        public boolean equals(Object obj)
+        {
+            if (obj instanceof DocumentIteratorEntry otherEntry) {
+                if (obj == this) {
+                    return true;
+                }
+
+                EqualsBuilder builder = new EqualsBuilder();
+
+                builder.append(getWiki(), otherEntry.getWiki());
+                builder.append(getDocId(), otherEntry.getDocId());
+                builder.append(getVersion(), otherEntry.getVersion());
+
+                return builder.build();
+            }
+
+            return false;
+        }
+
+        @Override
+        public String toString()
+        {
+            XWikiToStringBuilder builder = new XWikiToStringBuilder(this);
+
+            builder.append("wiki", getWiki());
+            builder.append("docId", getDocId());
+            builder.append("version", getVersion());
+
+            return builder.build();
+        }
+    }
+
     /**
      * Specifies the root entity whose documents are iterated. If {@code null} then all the documents are iterated.
      */

xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/job/DatabaseDocumentIterator.java

@@ -47,6 +47,7 @@
 import org.xwiki.query.QueryException;
 import org.xwiki.query.QueryFilter;
 import org.xwiki.query.QueryManager;
+import org.xwiki.search.solr.internal.job.AbstractDocumentIterator.DocumentIteratorEntry;
 import org.xwiki.wiki.descriptor.WikiDescriptorManager;
 import org.xwiki.wiki.manager.WikiManagerException;
 
@@ -59,7 +60,7 @@
 @Component
 @InstantiationStrategy(ComponentInstantiationStrategy.PER_LOOKUP)
 @Named("database")
-public class DatabaseDocumentIterator extends AbstractDocumentIterator<String>
+public class DatabaseDocumentIterator extends AbstractDocumentIterator<DocumentIteratorEntry>
 {
     /**
      * The current index in the list of {@link #results}.
@@ -130,20 +131,24 @@ public boolean hasNext()
     }
 
     @Override
-    public Pair<DocumentReference, String> next()
+    public Pair<DocumentReference, DocumentIteratorEntry> next()
     {
         Object[] result = getResults().get(index++);
+
         String localSpaceReference = (String) result[0];
         String name = (String) result[1];
         String locale = (String) result[2];
-        String version = (String) result[3];
         SpaceReference spaceReference = new SpaceReference(this.explicitEntityReferenceResolver
             .resolve(localSpaceReference, EntityType.SPACE, new WikiReference(wiki)));
         DocumentReference documentReference = new DocumentReference(name, spaceReference);
         if (!StringUtils.isEmpty(locale)) {
             documentReference = new DocumentReference(documentReference, LocaleUtils.toLocale(locale));
         }
-        return new ImmutablePair<DocumentReference, String>(documentReference, version);
+        String version = (String) result[3];
+        long docId = (long) result[4];
+
+        return new ImmutablePair<>(documentReference,
+            new DocumentIteratorEntry(documentReference.getWikiReference(), docId, version));
     }
 
     @Override

xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/job/DiffDocumentIterator.java

@@ -20,24 +20,19 @@
 package org.xwiki.search.solr.internal.job;
 
 import java.util.Comparator;
-import java.util.Objects;
 
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
-import org.xwiki.model.internal.reference.DefaultSymbolScheme;
-import org.xwiki.model.internal.reference.LocalStringEntityReferenceSerializer;
 import org.xwiki.model.reference.DocumentReference;
 import org.xwiki.model.reference.EntityReference;
-import org.xwiki.model.reference.EntityReferenceSerializer;
 
 /**
  * Compares the list of document references from two document iterators.
  * 
- * @param <T> the type of data used to determine if a document is up to date
  * @version $Id$
  * @since 5.4.5
  */
-public class DiffDocumentIterator<T> extends AbstractDocumentIterator<DiffDocumentIterator.Action>
+public class DiffDocumentIterator extends AbstractDocumentIterator<DiffDocumentIterator.Action>
 {
     /**
      * The action that must be taken in order to move from the previous version to the next version.
@@ -60,27 +55,27 @@ public enum Action
     /**
      * The document iterator that corresponds to the previous state (the store that needs to be updated).
      */
-    private final DocumentIterator<T> previous;
+    private final DocumentIterator<DocumentIteratorEntry> previous;
 
     /**
      * The document iterator that corresponds to the next state (the store that is used as a reference point).
      */
-    private final DocumentIterator<T> next;
+    private final DocumentIterator<DocumentIteratorEntry> next;
 
     /**
      * Used to compare document references.
      */
-    private final Comparator<DocumentReference> documentReferenceComparator = getComparator();
+    private final Comparator<DocumentIteratorEntry> entryComparator = getComparator();
 
     /**
      * The last entry taken from the {@link #previous} iterator.
      */
-    private Pair<DocumentReference, T> previousEntry;
+    private Pair<DocumentReference, DocumentIteratorEntry> previousEntry;
 
     /**
      * The last entry taken from the {@link #next} iterator.
      */
-    private Pair<DocumentReference, T> nextEntry;
+    private Pair<DocumentReference, DocumentIteratorEntry> nextEntry;
 
     /**
      * The last compare result between {@link #previousEntry} and {@link #nextEntry}.
@@ -94,7 +89,8 @@ public enum Action
      * @param next the document iterator that corresponds to the next state (the store that is used as a reference
      *            point)
      */
-    public DiffDocumentIterator(DocumentIterator<T> previous, DocumentIterator<T> next)
+    public DiffDocumentIterator(DocumentIterator<DocumentIteratorEntry> previous,
+        DocumentIterator<DocumentIteratorEntry> next)
     {
         this.previous = previous;
         this.next = next;
@@ -125,7 +121,7 @@ public Pair<DocumentReference, Action> next()
             if (diff <= 0) {
                 previousEntry = previous.next();
             }
-            diff = documentReferenceComparator.compare(previousEntry.getKey(), nextEntry.getKey());
+            diff = entryComparator.compare(previousEntry.getValue(), nextEntry.getValue());
             if (diff == 0) {
                 documentReference = nextEntry.getKey();
                 // Compare the document version.
@@ -157,15 +153,9 @@ public Pair<DocumentReference, Action> next()
      *
      * @return the comparator for comparing document references
      */
-    public static Comparator<DocumentReference> getComparator()
+    public static Comparator<DocumentIteratorEntry> getComparator()
     {
-        EntityReferenceSerializer<String> localEntityReferenceSerializer =
-            new LocalStringEntityReferenceSerializer(new DefaultSymbolScheme());
-        return Comparator.comparing(DocumentReference::getWikiReference)
-            // Compare by the whole space as string as this is also what we compare in the database and in Solr.
-            .thenComparing(documentReference -> localEntityReferenceSerializer.serialize(documentReference.getParent()))
-            .thenComparing(DocumentReference::getName)
-            .thenComparing(documentReference -> Objects.toString(documentReference.getLocale(), ""));
+        return Comparator.comparing(DocumentIteratorEntry::getWiki).thenComparing(DocumentIteratorEntry::getDocId);
     }
 
     @Override

xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/job/IndexerJob.java

@@ -42,6 +42,7 @@
 import org.xwiki.search.solr.internal.api.SolrIndexer;
 import org.xwiki.search.solr.internal.api.SolrIndexerException;
 import org.xwiki.search.solr.internal.api.SolrInstance;
+import org.xwiki.search.solr.internal.job.AbstractDocumentIterator.DocumentIteratorEntry;
 import org.xwiki.search.solr.internal.job.DiffDocumentIterator.Action;
 import org.xwiki.search.solr.internal.reference.SolrReferenceResolver;
 
@@ -74,11 +75,11 @@ public class IndexerJob extends AbstractJob<IndexerRequest, DefaultJobStatus<Ind
 
     @Inject
     @Named("database")
-    private transient DocumentIterator<String> databaseIterator;
+    private transient DocumentIterator<DocumentIteratorEntry> databaseIterator;
 
     @Inject
     @Named("solr")
-    private transient DocumentIterator<String> solrIterator;
+    private transient DocumentIterator<DocumentIteratorEntry> solrIterator;
 
     @Inject
     private EntityReferenceSerializer<String> entityReferenceSerializer;
@@ -125,7 +126,7 @@ protected void runInternal() throws Exception
      */
     private void updateSolrIndex() throws Exception
     {
-        DiffDocumentIterator<String> iterator = new DiffDocumentIterator<>(this.solrIterator, this.databaseIterator);
+        DiffDocumentIterator iterator = new DiffDocumentIterator(this.solrIterator, this.databaseIterator);
         iterator.setRootReference(getRequest().getRootReference());
 
         this.progressManager.pushLevelProgress(2, this);
@@ -147,7 +148,7 @@ private void updateSolrIndex() throws Exception
         }
     }
 
-    private void updateSolrIndex(int progressSize, DiffDocumentIterator<String> iterator) throws Exception
+    private void updateSolrIndex(int progressSize, DiffDocumentIterator iterator) throws Exception
     {
         this.progressManager.pushLevelProgress(progressSize, this);
 

xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-api/src/main/java/org/xwiki/search/solr/internal/job/SolrDocumentIterator.java

@@ -42,6 +42,7 @@
 import org.xwiki.search.solr.internal.api.FieldUtils;
 import org.xwiki.search.solr.internal.api.SolrIndexerException;
 import org.xwiki.search.solr.internal.api.SolrInstance;
+import org.xwiki.search.solr.internal.job.AbstractDocumentIterator.DocumentIteratorEntry;
 import org.xwiki.search.solr.internal.reference.SolrReferenceResolver;
 
 /**
@@ -53,7 +54,7 @@
 @Component
 @InstantiationStrategy(ComponentInstantiationStrategy.PER_LOOKUP)
 @Named("solr")
-public class SolrDocumentIterator extends AbstractDocumentIterator<String>
+public class SolrDocumentIterator extends AbstractDocumentIterator<DocumentIteratorEntry>
 {
     private static final String SOLR_ANYVALUE = "[* TO *]";
 
@@ -94,12 +95,16 @@ public boolean hasNext()
     }
 
     @Override
-    public Pair<DocumentReference, String> next()
+    public Pair<DocumentReference, DocumentIteratorEntry> next()
     {
-        SolrDocument result = getResults().get(index++);
+        SolrDocument result = getResults().get(this.index++);
+
         DocumentReference documentReference = this.solrDocumentReferenceResolver.resolve(result);
+        long docId = (long) result.getFieldValue(FieldUtils.DOC_ID);
         String version = (String) result.get(FieldUtils.VERSION);
-        return new ImmutablePair<DocumentReference, String>(documentReference, version);
+
+        return new ImmutablePair<>(documentReference,
+            new DocumentIteratorEntry(documentReference.getWikiReference(), docId, version));
     }
 
     @Override
@@ -144,7 +149,7 @@ private SolrQuery getQuery() throws SolrIndexerException
         if (query == null) {
             query = new SolrQuery(solrReferenceResolver.getQuery(rootReference));
             query.setFields(FieldUtils.WIKI, FieldUtils.SPACES, FieldUtils.NAME, FieldUtils.DOCUMENT_LOCALE,
-                FieldUtils.VERSION);
+                FieldUtils.VERSION, FieldUtils.DOC_ID);
             query.addFilterQuery(FieldUtils.TYPE + ':' + EntityType.DOCUMENT.name());
             // Make sure to skip invalid documents, they will be re-indexed
             query.addFilterQuery(FieldUtils.WIKI + ':' + SOLR_ANYVALUE);