From ceba520778bff3e72c7f1e5f43f6a86f62613192 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Mon, 18 Sep 2023 14:28:07 -0400 Subject: [PATCH 1/8] optimize udvas by reusing dv iterators --- .../component/RealTimeGetComponent.java | 14 +- .../solr/search/DocValuesIteratorCache.java | 309 ++++++++++++++++++ .../solr/search/SolrDocumentFetcher.java | 74 +++-- 3 files changed, 363 insertions(+), 34 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java diff --git a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java index 56aebd8e9d2..9324d99685d 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java @@ -81,6 +81,7 @@ import org.apache.solr.schema.IndexSchema; import org.apache.solr.schema.SchemaField; import org.apache.solr.search.DocList; +import org.apache.solr.search.DocValuesIteratorCache; import org.apache.solr.search.QueryUtils; import org.apache.solr.search.ReturnFields; import org.apache.solr.search.SolrDocumentFetcher; @@ -238,6 +239,7 @@ public void process(ResponseBuilder rb) throws IOException { boolean opennedRealtimeSearcher = false; BytesRefBuilder idBytes = new BytesRefBuilder(); + DocValuesIteratorCache reuseDvIters = null; for (String idStr : reqIds.allIds) { fieldType.readableToIndexed(idStr, idBytes); // if _route_ is passed, id is a child doc. TODO remove in SOLR-15064 @@ -348,7 +350,11 @@ public void process(ResponseBuilder rb) throws IOException { searcherInfo.getSearcher().doc(docid, rsp.getReturnFields().getLuceneFieldNames()); SolrDocument doc = toSolrDoc(luceneDocument, core.getLatestSchema()); SolrDocumentFetcher docFetcher = searcherInfo.getSearcher().getDocFetcher(); - docFetcher.decorateDocValueFields(doc, docid, docFetcher.getNonStoredDVs(true)); + if (reuseDvIters == null) { + reuseDvIters = new DocValuesIteratorCache(searcherInfo.getSearcher()); + } + docFetcher.decorateDocValueFields( + doc, docid, docFetcher.getNonStoredDVs(true), reuseDvIters); if (null != transformer) { if (null == resultContext) { // either first pass, or we've re-opened searcher - either way now we setContext @@ -575,7 +581,11 @@ private static SolrDocument mergePartialDocWithFullDocFromIndex( if (!doc.containsKey(VERSION_FIELD)) { searcher .getDocFetcher() - .decorateDocValueFields(doc, docid, Collections.singleton(VERSION_FIELD)); + .decorateDocValueFields( + doc, + docid, + Collections.singleton(VERSION_FIELD), + new DocValuesIteratorCache(searcher, false)); } long docVersion = (long) doc.getFirstValue(VERSION_FIELD); diff --git a/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java new file mode 100644 index 00000000000..8ab989100fb --- /dev/null +++ b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java @@ -0,0 +1,309 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.search; + +import java.io.IOException; +import java.util.Arrays; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.function.Function; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.solr.schema.SchemaField; + +/** + * A helper class for random-order value access over docValues (such as in the case of + * useDocValuesAsStored). This class optimizes access by reusing DocValues iterators where possible, + * and by narrowing the scope of DocValues per-field/per-segment (shortcircuiting attempts to + * `advance()` to docs that are known to have no value for a given field). + */ +public class DocValuesIteratorCache { + + private static final EnumMap> + funcMap = new EnumMap<>(DocValuesType.class); + + static { + funcMap.put(DocValuesType.NUMERIC, LeafReader::getNumericDocValues); + funcMap.put(DocValuesType.BINARY, LeafReader::getBinaryDocValues); + funcMap.put( + DocValuesType.SORTED, + (r, f) -> { + SortedDocValues dvs = r.getSortedDocValues(f); + return dvs == null || dvs.getValueCount() < 1 ? null : dvs; + }); + funcMap.put(DocValuesType.SORTED_NUMERIC, LeafReader::getSortedNumericDocValues); + funcMap.put( + DocValuesType.SORTED_SET, + (r, f) -> { + SortedSetDocValues dvs = r.getSortedSetDocValues(f); + return dvs == null || dvs.getValueCount() < 1 ? null : dvs; + }); + } + + private static final FieldDocValuesSupplier NONE = new FieldDocValuesSupplier(null, null, 0); + + private final SolrIndexSearcher searcher; + private final int nLeaves; + private final Function getSupplier; + + /** + * Construct an instance used to optimize random-order DocValues iterator access for the specified + * searcher. + */ + public DocValuesIteratorCache(SolrIndexSearcher searcher) { + this(searcher, true); + } + + /** + * Construct an instance used to optimize random-order DocValues iterator access for the specified + * searcher. + * + * @param searcher the associated searcher + * @param cache if false, caching is disabled (useful mainly for single-field, single-doc access). + */ + public DocValuesIteratorCache(SolrIndexSearcher searcher, boolean cache) { + this.searcher = searcher; + this.nLeaves = searcher.getTopReaderContext().leaves().size(); + if (cache) { + HashMap map = new HashMap<>(); + getSupplier = (f) -> map.computeIfAbsent(f, this::newEntry); + } else { + getSupplier = this::newEntry; + } + } + + public FieldDocValuesSupplier getSupplier(String fieldName) { + FieldDocValuesSupplier ret = getSupplier.apply(fieldName); + return ret == NONE ? null : ret; + } + + private FieldDocValuesSupplier newEntry(String fieldName) { + final SchemaField schemaField = searcher.getSchema().getFieldOrNull(fieldName); + FieldInfo fi = searcher.getFieldInfos().fieldInfo(fieldName); + if (schemaField == null || !schemaField.hasDocValues() || fi == null) { + return NONE; // Searcher doesn't have info about this field, hence ignore it. + } + final DocValuesType dvType = fi.getDocValuesType(); + switch (dvType) { + case NUMERIC: + case BINARY: + case SORTED: + case SORTED_NUMERIC: + case SORTED_SET: + return new FieldDocValuesSupplier(schemaField, dvType, nLeaves); + default: + return NONE; + } + } + + private interface IOBiFunction { + R apply(T t, U u) throws IOException; + } + + /** + * Supplies (and coordinates arbitrary-order value retrieval over) docValues iterators for a + * particular field, encapsulating the logic of iterator creation, reuse/caching, and advancing. + * Returned iterators are already positioned, and should not be advanced (though + * multi-valued iterators may consume/iterate over values/ords). + * + *

Instances of this class are specifically designed to support arbitrary-order value + * retrieval, (e.g., useDocValuesAsStored, ExportWriter) and should generally not be used for + * ordered retrieval (although ordered retrieval would work perfectly fine, and would add only + * minimal overhead). + */ + public static class FieldDocValuesSupplier { + public final SchemaField schemaField; + public final DocValuesType type; + private final int[] minLocalIds; + private final int[] ceilingIds; + private final int[] noMatchSince; + private final DocIdSetIterator[] perLeaf; + + private FieldDocValuesSupplier(SchemaField schemaField, DocValuesType type, int nLeaves) { + this.schemaField = schemaField; + this.type = type; + this.minLocalIds = new int[nLeaves]; + Arrays.fill(minLocalIds, -1); + this.ceilingIds = new int[nLeaves]; + Arrays.fill(ceilingIds, DocIdSetIterator.NO_MORE_DOCS); + this.noMatchSince = new int[nLeaves]; + this.perLeaf = new DocIdSetIterator[nLeaves]; + } + + /** + * This method does the actual work caching iterators, determining eligibility for re-use, + * pulling new iterators if necessary, and determining if we have a hit for a particular doc id. + */ + private DocIdSetIterator getDocValues( + int localId, + LeafReader leafReader, + int leafOrd, + boolean singleValued, + IOBiFunction dvFunction) + throws IOException { + int min = minLocalIds[leafOrd]; + DocIdSetIterator dv; + if (min == -1) { + // we are not yet initialized for this field/leaf. + dv = dvFunction.apply(leafReader, schemaField.getName()); + if (dv == null) { + minLocalIds[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; // cache absence of this field + return null; + } + // on field/leaf init, determine the min doc, so that we don't expend effort pulling + // new iterators for docs that fall below this floor. + min = dv.nextDoc(); + minLocalIds[leafOrd] = min; + perLeaf[leafOrd] = dv; + if (localId < min) { + noMatchSince[leafOrd] = 0; // implicit in initial `nextDoc()` call + return null; + } else if (localId == min) { + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; + return dv; + } + } else if (localId < min || localId >= ceilingIds[leafOrd]) { + // out of range: either too low or too high + return null; + } else { + dv = perLeaf[leafOrd]; + int currentDoc = dv.docID(); + if (localId == currentDoc + && (singleValued || noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS)) { + // `noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS` means that `dv` has not + // been returned at its current position, and has therefore not been consumed and + // is thus eligible to be returned directly. (singleValued dv iterators are always + // eligible to be returned directly, as they have no concept of being "consumed") + return dv; + } else if (localId <= currentDoc) { + if (localId >= noMatchSince[leafOrd]) { + // if the requested doc falls between the last requested doc and the current + // position, then we know there's no match. + return null; + } + // we must re-init the iterator + dv = dvFunction.apply(leafReader, schemaField.getName()); + perLeaf[leafOrd] = dv; + } + } + // NOTE: use `advance()`, not `advanceExact()`. There's no cost (in terms of re-use) to + // doing so, because we track `noMatchSince` in the event of a miss. + int found = dv.advance(localId); + if (found == localId) { + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; + return dv; + } else { + if (found == DocIdSetIterator.NO_MORE_DOCS) { + ceilingIds[leafOrd] = Math.min(localId, ceilingIds[leafOrd]); + } + noMatchSince[leafOrd] = localId; + return null; + } + } + + /** + * Returns docValues for the specified doc id in the specified reader, if the specified doc + * holds docValues for this {@link FieldDocValuesSupplier} instance, otherwise returns null. + * + *

If a non-null value is returned, it will already positioned at the specified docId. + * + * @param localId leaf-scoped docId + * @param leafReader reader containing docId + * @param leafOrd top-level ord of the specified reader + */ + public NumericDocValues getNumericDocValues(int localId, LeafReader leafReader, int leafOrd) + throws IOException { + return (NumericDocValues) + getDocValues(localId, leafReader, leafOrd, true, funcMap.get(DocValuesType.NUMERIC)); + } + + /** + * Returns docValues for the specified doc id in the specified reader, if the specified doc + * holds docValues for this {@link FieldDocValuesSupplier} instance, otherwise returns null. + * + *

If a non-null value is returned, it will already positioned at the specified docId. + * + * @param localId leaf-scoped docId + * @param leafReader reader containing docId + * @param leafOrd top-level ord of the specified reader + */ + public BinaryDocValues getBinaryDocValues(int localId, LeafReader leafReader, int leafOrd) + throws IOException { + return (BinaryDocValues) + getDocValues(localId, leafReader, leafOrd, true, funcMap.get(DocValuesType.BINARY)); + } + + /** + * Returns docValues for the specified doc id in the specified reader, if the specified doc + * holds docValues for this {@link FieldDocValuesSupplier} instance, otherwise returns null. + * + *

If a non-null value is returned, it will already positioned at the specified docId. + * + * @param localId leaf-scoped docId + * @param leafReader reader containing docId + * @param leafOrd top-level ord of the specified reader + */ + public SortedDocValues getSortedDocValues(int localId, LeafReader leafReader, int leafOrd) + throws IOException { + return (SortedDocValues) + getDocValues(localId, leafReader, leafOrd, true, funcMap.get(DocValuesType.SORTED)); + } + + /** + * Returns docValues for the specified doc id in the specified reader, if the specified doc + * holds docValues for this {@link FieldDocValuesSupplier} instance, otherwise returns null. + * + *

If a non-null value is returned, it will already positioned at the specified docId, and + * with values ({@link SortedNumericDocValues#nextValue()}) not yet consumed. + * + * @param localId leaf-scoped docId + * @param leafReader reader containing docId + * @param leafOrd top-level ord of the specified reader + */ + public SortedNumericDocValues getSortedNumericDocValues( + int localId, LeafReader leafReader, int leafOrd) throws IOException { + return (SortedNumericDocValues) + getDocValues( + localId, leafReader, leafOrd, false, funcMap.get(DocValuesType.SORTED_NUMERIC)); + } + + /** + * Returns docValues for the specified doc id in the specified reader, if the specified doc + * holds docValues for this {@link FieldDocValuesSupplier} instance, otherwise returns null. + * + *

If a non-null value is returned, it will already positioned at the specified docId, and + * with ords ({@link SortedSetDocValues#nextOrd()}) not yet consumed. + * + * @param localId leaf-scoped docId + * @param leafReader reader containing docId + * @param leafOrd top-level ord of the specified reader + */ + public SortedSetDocValues getSortedSetDocValues(int localId, LeafReader leafReader, int leafOrd) + throws IOException { + return (SortedSetDocValues) + getDocValues(localId, leafReader, leafOrd, false, funcMap.get(DocValuesType.SORTED_SET)); + } + } +} diff --git a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java index a975337f869..e6ccb9edd18 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java @@ -84,6 +84,8 @@ public class SolrDocumentFetcher { private final SolrIndexSearcher searcher; + private final int nLeaves; + private final boolean enableLazyFieldLoading; private final SolrCache documentCache; @@ -119,6 +121,7 @@ public class SolrDocumentFetcher { @SuppressWarnings({"unchecked"}) SolrDocumentFetcher(SolrIndexSearcher searcher, SolrConfig solrConfig, boolean cachingEnabled) { this.searcher = searcher; + this.nLeaves = searcher.getTopReaderContext().leaves().size(); this.enableLazyFieldLoading = solrConfig.enableLazyFieldLoading; if (cachingEnabled) { documentCache = @@ -561,16 +564,23 @@ public InvertableType invertableType() { * @param fields The fields with docValues to populate the document with. DocValues fields which * do not exist or not decodable will be ignored. */ - public void decorateDocValueFields(SolrDocumentBase doc, int docid, Set fields) + public void decorateDocValueFields( + SolrDocumentBase doc, + int docid, + Set fields, + DocValuesIteratorCache reuseDvIters) throws IOException { final List leafContexts = searcher.getLeafContexts(); final int subIndex = ReaderUtil.subIndex(docid, leafContexts); final int localId = docid - leafContexts.get(subIndex).docBase; final LeafReader leafReader = leafContexts.get(subIndex).reader(); for (String fieldName : fields) { - Object fieldValue = decodeDVField(localId, leafReader, fieldName); - if (fieldValue != null) { - doc.setField(fieldName, fieldValue); + DocValuesIteratorCache.FieldDocValuesSupplier e = reuseDvIters.getSupplier(fieldName); + if (e != null) { + Object fieldValue = decodeDVField(localId, leafReader, subIndex, e); + if (fieldValue != null) { + doc.setField(fieldName, fieldValue); + } } } } @@ -580,59 +590,56 @@ public void decorateDocValueFields(SolrDocumentBase doc, int docid, Set outValues = new ArrayList<>(docValueCount); for (int i = 0; i < docValueCount; i++) { long number = numericDv.nextValue(); - Object value = decodeNumberFromDV(schemaField, number, true); + Object value = decodeNumberFromDV(e.schemaField, number, true); // return immediately if the number is not decodable, hence won't return an empty list. if (value == null) { return null; } // normally never true but LatLonPointSpatialField uses SORTED_NUMERIC even when single // valued - else if (schemaField.multiValued() == false) { + else if (e.schemaField.multiValued() == false) { return value; } else { outValues.add(value); @@ -643,21 +650,21 @@ else if (schemaField.multiValued() == false) { } return null; case SORTED_SET: - final SortedSetDocValues values = leafReader.getSortedSetDocValues(fieldName); - if (values != null && values.getValueCount() > 0 && values.advance(localId) == localId) { + final SortedSetDocValues values = e.getSortedSetDocValues(localId, leafReader, readerOrd); + if (values != null) { final List outValues = new ArrayList<>(); for (long ord = values.nextOrd(); ord != SortedSetDocValues.NO_MORE_ORDS; ord = values.nextOrd()) { BytesRef value = values.lookupOrd(ord); - outValues.add(schemaField.getType().toObject(schemaField, value)); + outValues.add(e.schemaField.getType().toObject(e.schemaField, value)); } assert outValues.size() > 0; return outValues; } return null; default: - return null; + throw new IllegalStateException(); } } @@ -751,6 +758,8 @@ class RetrieveFieldsOptimizer { private final SolrReturnFields solrReturnFields; + private final DocValuesIteratorCache reuseDvIters; + RetrieveFieldsOptimizer(SolrReturnFields solrReturnFields) { this.storedFields = calcStoredFieldsForReturn(solrReturnFields); this.dvFields = calcDocValueFieldsForReturn(solrReturnFields); @@ -760,6 +769,7 @@ class RetrieveFieldsOptimizer { dvFields.addAll(storedFields); storedFields.clear(); } + reuseDvIters = dvFields.isEmpty() ? null : new DocValuesIteratorCache(searcher); } /** @@ -881,7 +891,7 @@ private SolrDocument getSolrDoc(int luceneDocId) { // decorate the document with non-stored docValues fields if (returnDVFields()) { - decorateDocValueFields(sdoc, luceneDocId, getDvFields()); + decorateDocValueFields(sdoc, luceneDocId, getDvFields(), reuseDvIters); } } catch (IOException e) { throw new SolrException( From 928fcaa2b15c39cd1b5ee1f3b9a40f087e49f186 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Tue, 19 Sep 2023 10:09:31 -0400 Subject: [PATCH 2/8] apply to export writers --- .../solr/handler/export/BoolFieldWriter.java | 5 +- .../solr/handler/export/DateFieldWriter.java | 30 +++------ .../handler/export/DoubleFieldWriter.java | 29 +++------ .../solr/handler/export/ExportWriter.java | 30 +++++---- .../solr/handler/export/FloatFieldWriter.java | 30 +++------ .../solr/handler/export/IntFieldWriter.java | 30 +++------ .../solr/handler/export/LongFieldWriter.java | 30 +++------ .../solr/handler/export/MultiFieldWriter.java | 64 +++++++------------ .../handler/export/StringFieldWriter.java | 31 +++------ 9 files changed, 96 insertions(+), 183 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java index fbeccdc4c0d..ea74856dcad 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java @@ -21,10 +21,11 @@ import org.apache.lucene.util.BytesRef; import org.apache.solr.common.MapWriter; import org.apache.solr.schema.FieldType; +import org.apache.solr.search.DocValuesIteratorCache; class BoolFieldWriter extends StringFieldWriter { - public BoolFieldWriter(String field, FieldType fieldType) { - super(field, fieldType); + public BoolFieldWriter(String field, FieldType fieldType, DocValuesIteratorCache dvIterCache) { + super(field, fieldType, dvIterCache); } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java index c86f0eacac1..14861f83c63 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java @@ -17,20 +17,20 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; import java.util.Date; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; +import org.apache.solr.search.DocValuesIteratorCache; class DateFieldWriter extends FieldWriter { - private String field; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final String field; + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public DateFieldWriter(String field) { + public DateFieldWriter(String field, DocValuesIteratorCache dvIterCache) { this.field = field; + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -47,22 +47,10 @@ public boolean write( } } else { // field is not part of 'sort' param, but part of 'fl' param - int readerOrd = readerContext.ord; - NumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - NumericDocValues numericDocValues = docValuesCache.get(readerOrd); - if (numericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = numericDocValues; - } - } - - if (vals == null) { - vals = DocValues.getNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - - if (vals.advance(sortDoc.docId) == sortDoc.docId) { + NumericDocValues vals = + docValuesCache.getNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); + if (vals != null) { val = vals.longValue(); } else { return false; diff --git a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java index 50cbdddb385..2ccc75e7ed3 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java @@ -17,19 +17,19 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; +import org.apache.solr.search.DocValuesIteratorCache; class DoubleFieldWriter extends FieldWriter { - private String field; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final String field; + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public DoubleFieldWriter(String field) { + public DoubleFieldWriter(String field, DocValuesIteratorCache dvIterCache) { this.field = field; + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -47,21 +47,10 @@ public boolean write( } } else { // field is not part of 'sort' param, but part of 'fl' param - int readerOrd = readerContext.ord; - NumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - NumericDocValues numericDocValues = docValuesCache.get(readerOrd); - if (numericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = numericDocValues; - } - } - - if (vals == null) { - vals = DocValues.getNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - if (vals.advance(sortDoc.docId) == sortDoc.docId) { + NumericDocValues vals = + docValuesCache.getNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); + if (vals != null) { long val = vals.longValue(); ew.put(this.field, Double.longBitsToDouble(val)); return true; diff --git a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java index 8f38c94942e..22a27983009 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java @@ -74,6 +74,7 @@ import org.apache.solr.schema.SchemaField; import org.apache.solr.schema.SortableTextField; import org.apache.solr.schema.StrField; +import org.apache.solr.search.DocValuesIteratorCache; import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.search.SortSpec; import org.apache.solr.search.SyntaxError; @@ -480,6 +481,7 @@ public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher throws IOException { IndexSchema schema = searcher.getSchema(); FieldWriter[] writers = new FieldWriter[fields.length]; + DocValuesIteratorCache dvIterCache = new DocValuesIteratorCache(searcher, false); for (int i = 0; i < fields.length; i++) { String field = fields[i]; SchemaField schemaField = null; @@ -503,45 +505,45 @@ public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher if (fieldType instanceof IntValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); } else { - writers[i] = new IntFieldWriter(field); + writers[i] = new IntFieldWriter(field, dvIterCache); } } else if (fieldType instanceof LongValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); } else { - writers[i] = new LongFieldWriter(field); + writers[i] = new LongFieldWriter(field, dvIterCache); } } else if (fieldType instanceof FloatValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); } else { - writers[i] = new FloatFieldWriter(field); + writers[i] = new FloatFieldWriter(field, dvIterCache); } } else if (fieldType instanceof DoubleValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); } else { - writers[i] = new DoubleFieldWriter(field); + writers[i] = new DoubleFieldWriter(field, dvIterCache); } } else if (fieldType instanceof StrField || fieldType instanceof SortableTextField) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, dvIterCache); } else { - writers[i] = new StringFieldWriter(field, fieldType); + writers[i] = new StringFieldWriter(field, fieldType, dvIterCache); } } else if (fieldType instanceof DateValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, dvIterCache); } else { - writers[i] = new DateFieldWriter(field); + writers[i] = new DateFieldWriter(field, dvIterCache); } } else if (fieldType instanceof BoolField) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); } else { - writers[i] = new BoolFieldWriter(field, fieldType); + writers[i] = new BoolFieldWriter(field, fieldType, dvIterCache); } } else { throw new IOException( diff --git a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java index 48bd9d632ed..dd074f39173 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java @@ -17,19 +17,19 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; +import org.apache.solr.search.DocValuesIteratorCache; class FloatFieldWriter extends FieldWriter { - private String field; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final String field; + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public FloatFieldWriter(String field) { + public FloatFieldWriter(String field, DocValuesIteratorCache dvIterCache) { this.field = field; + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -47,22 +47,10 @@ public boolean write( } } else { // field is not part of 'sort' param, but part of 'fl' param - int readerOrd = readerContext.ord; - NumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - NumericDocValues numericDocValues = docValuesCache.get(readerOrd); - if (numericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = numericDocValues; - } - } - - if (vals == null) { - vals = DocValues.getNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - - if (vals.advance(sortDoc.docId) == sortDoc.docId) { + NumericDocValues vals = + docValuesCache.getNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); + if (vals != null) { int val = (int) vals.longValue(); ew.put(this.field, Float.intBitsToFloat(val)); return true; diff --git a/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java index 1ecef85f21b..0a2e107efe1 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java @@ -17,19 +17,19 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; +import org.apache.solr.search.DocValuesIteratorCache; class IntFieldWriter extends FieldWriter { - private String field; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final String field; + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public IntFieldWriter(String field) { + public IntFieldWriter(String field, DocValuesIteratorCache dvIterCache) { this.field = field; + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -46,22 +46,10 @@ public boolean write( } } else { // field is not part of 'sort' param, but part of 'fl' param - int readerOrd = readerContext.ord; - NumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - NumericDocValues numericDocValues = docValuesCache.get(readerOrd); - if (numericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = numericDocValues; - } - } - - if (vals == null) { - vals = DocValues.getNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - - if (vals.advance(sortDoc.docId) == sortDoc.docId) { + NumericDocValues vals = + docValuesCache.getNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); + if (vals != null) { val = (int) vals.longValue(); } else { return false; diff --git a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java index 9c18a72bd6d..ce42467cda0 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java @@ -17,20 +17,20 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; +import org.apache.solr.search.DocValuesIteratorCache; class LongFieldWriter extends FieldWriter { - private String field; + private final String field; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public LongFieldWriter(String field) { + public LongFieldWriter(String field, DocValuesIteratorCache dvIterCache) { this.field = field; + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -47,22 +47,10 @@ public boolean write( } } else { // field is not part of 'sort' param, but part of 'fl' param - int readerOrd = readerContext.ord; - NumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - NumericDocValues numericDocValues = docValuesCache.get(readerOrd); - if (numericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = numericDocValues; - } - } - - if (vals == null) { - vals = DocValues.getNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - - if (vals.advance(sortDoc.docId) == sortDoc.docId) { + NumericDocValues vals = + docValuesCache.getNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); + if (vals != null) { val = vals.longValue(); } else { return false; diff --git a/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java index 86dd8ba82e3..d64fa5d0605 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java @@ -17,11 +17,9 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; import java.util.Date; import java.util.function.LongFunction; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.SortedNumericDocValues; @@ -33,18 +31,23 @@ import org.apache.solr.common.MapWriter; import org.apache.solr.schema.FieldType; import org.apache.solr.schema.SchemaField; +import org.apache.solr.search.DocValuesIteratorCache; class MultiFieldWriter extends FieldWriter { - private String field; - private FieldType fieldType; - private SchemaField schemaField; - private boolean numeric; - private CharsRefBuilder cref = new CharsRefBuilder(); + private final String field; + private final FieldType fieldType; + private final SchemaField schemaField; + private final boolean numeric; + private final CharsRefBuilder cref = new CharsRefBuilder(); private final LongFunction bitsToValue; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; public MultiFieldWriter( - String field, FieldType fieldType, SchemaField schemaField, boolean numeric) { + String field, + FieldType fieldType, + SchemaField schemaField, + boolean numeric, + DocValuesIteratorCache dvIterCache) { this.field = field; this.fieldType = fieldType; this.schemaField = schemaField; @@ -54,6 +57,7 @@ public MultiFieldWriter( } else { bitsToValue = null; } + docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -61,24 +65,13 @@ public boolean write( SortDoc sortDoc, LeafReaderContext readerContext, MapWriter.EntryWriter out, int fieldIndex) throws IOException { if (this.fieldType.isPointField()) { - int readerOrd = readerContext.ord; - SortedNumericDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - SortedNumericDocValues sortedNumericDocValues = - (SortedNumericDocValues) docValuesCache.get(readerOrd); - if (sortedNumericDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = sortedNumericDocValues; - } - } - + SortedNumericDocValues vals = + docValuesCache.getSortedNumericDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); if (vals == null) { - vals = DocValues.getSortedNumeric(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); + return false; } - if (!vals.advanceExact(sortDoc.docId)) return false; - final SortedNumericDocValues docVals = vals; out.put( @@ -91,32 +84,21 @@ public boolean write( }); return true; } else { - int readerOrd = readerContext.ord; - SortedSetDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - SortedSetDocValues sortedSetDocValues = (SortedSetDocValues) docValuesCache.get(readerOrd); - if (sortedSetDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = sortedSetDocValues; - } - } - + SortedSetDocValues vals = + docValuesCache.getSortedSetDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); if (vals == null) { - vals = DocValues.getSortedSet(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); + return false; } - if (vals.advance(sortDoc.docId) != sortDoc.docId) return false; - final SortedSetDocValues docVals = vals; out.put( this.field, (IteratorWriter) w -> { - long o; - while ((o = docVals.nextOrd()) != SortedSetDocValues.NO_MORE_ORDS) { - BytesRef ref = docVals.lookupOrd(o); + for (int i = 0, count = docVals.docValueCount(); i < count; i++) { + BytesRef ref = docVals.lookupOrd(docVals.nextOrd()); fieldType.indexedToReadable(ref, cref); IndexableField f = fieldType.createField(schemaField, cref.toString()); if (f == null) w.add(cref.toString()); diff --git a/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java index 7e15704f32d..ffa30440108 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java @@ -17,9 +17,7 @@ package org.apache.solr.handler.export; -import com.carrotsearch.hppc.IntObjectHashMap; import java.io.IOException; -import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.util.BytesRef; @@ -28,13 +26,14 @@ import org.apache.solr.common.util.ByteArrayUtf8CharSequence; import org.apache.solr.common.util.JavaBinCodec; import org.apache.solr.schema.FieldType; +import org.apache.solr.search.DocValuesIteratorCache; class StringFieldWriter extends FieldWriter { - protected String field; - private FieldType fieldType; + protected final String field; + private final FieldType fieldType; private BytesRef lastRef; private int lastOrd = -1; - private IntObjectHashMap docValuesCache = new IntObjectHashMap<>(); + private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; protected CharsRefBuilder cref = new CharsRefBuilder(); final ByteArrayUtf8CharSequence utf8 = @@ -50,9 +49,10 @@ public String toString() { } }; - public StringFieldWriter(String field, FieldType fieldType) { + public StringFieldWriter(String field, FieldType fieldType, DocValuesIteratorCache dvIterCache) { this.field = field; this.fieldType = fieldType; + this.docValuesCache = dvIterCache.getSupplier(field); } @Override @@ -82,23 +82,10 @@ public boolean write( } if (ref == null) { - // Reuse the last DocValues object if possible - int readerOrd = readerContext.ord; - SortedDocValues vals = null; - if (docValuesCache.containsKey(readerOrd)) { - SortedDocValues sortedDocValues = docValuesCache.get(readerOrd); - if (sortedDocValues.docID() < sortDoc.docId) { - // We have not advanced beyond the current docId so we can use this docValues. - vals = sortedDocValues; - } - } - + SortedDocValues vals = + docValuesCache.getSortedDocValues( + sortDoc.docId, readerContext.reader(), readerContext.ord); if (vals == null) { - vals = DocValues.getSorted(readerContext.reader(), this.field); - docValuesCache.put(readerOrd, vals); - } - - if (vals.advance(sortDoc.docId) != sortDoc.docId) { return false; } From 4a562608b2db3a7efdfa0b917407437a185bcd1f Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Tue, 19 Sep 2023 10:45:37 -0400 Subject: [PATCH 3/8] increase consistency of field writers --- .../solr/handler/export/DateFieldWriter.java | 36 +++---------------- .../handler/export/DoubleFieldWriter.java | 11 +++--- .../solr/handler/export/FloatFieldWriter.java | 11 +++--- .../solr/handler/export/LongFieldWriter.java | 8 +++-- 4 files changed, 20 insertions(+), 46 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java index 14861f83c63..b0b9704ffb2 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java @@ -19,44 +19,16 @@ import java.io.IOException; import java.util.Date; -import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.NumericDocValues; import org.apache.solr.common.MapWriter; import org.apache.solr.search.DocValuesIteratorCache; -class DateFieldWriter extends FieldWriter { - private final String field; - private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - +class DateFieldWriter extends LongFieldWriter { public DateFieldWriter(String field, DocValuesIteratorCache dvIterCache) { - this.field = field; - docValuesCache = dvIterCache.getSupplier(field); + super(field, dvIterCache); } @Override - public boolean write( - SortDoc sortDoc, LeafReaderContext readerContext, MapWriter.EntryWriter ew, int fieldIndex) - throws IOException { - Long val; - SortValue sortValue = sortDoc.getSortValue(this.field); - if (sortValue != null) { - if (sortValue.isPresent()) { - val = (long) sortValue.getCurrentValue(); - } else { // empty-value - return false; - } - } else { - // field is not part of 'sort' param, but part of 'fl' param - NumericDocValues vals = - docValuesCache.getNumericDocValues( - sortDoc.docId, readerContext.reader(), readerContext.ord); - if (vals != null) { - val = vals.longValue(); - } else { - return false; - } - } - ew.put(this.field, new Date(val)); - return true; + protected void doWrite(MapWriter.EntryWriter ew, long val) throws IOException { + ew.put(field, new Date(val)); } } diff --git a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java index 2ccc75e7ed3..2dc81cafbd4 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java @@ -36,12 +36,11 @@ public DoubleFieldWriter(String field, DocValuesIteratorCache dvIterCache) { public boolean write( SortDoc sortDoc, LeafReaderContext readerContext, MapWriter.EntryWriter ew, int fieldIndex) throws IOException { + double val; SortValue sortValue = sortDoc.getSortValue(this.field); if (sortValue != null) { if (sortValue.isPresent()) { - double val = (double) sortValue.getCurrentValue(); - ew.put(this.field, val); - return true; + val = (double) sortValue.getCurrentValue(); } else { // empty-value return false; } @@ -51,12 +50,12 @@ public boolean write( docValuesCache.getNumericDocValues( sortDoc.docId, readerContext.reader(), readerContext.ord); if (vals != null) { - long val = vals.longValue(); - ew.put(this.field, Double.longBitsToDouble(val)); - return true; + val = Double.longBitsToDouble(vals.longValue()); } else { return false; } } + ew.put(this.field, val); + return true; } } diff --git a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java index dd074f39173..518b7391412 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java @@ -36,12 +36,11 @@ public FloatFieldWriter(String field, DocValuesIteratorCache dvIterCache) { public boolean write( SortDoc sortDoc, LeafReaderContext readerContext, MapWriter.EntryWriter ew, int fieldIndex) throws IOException { + float val; SortValue sortValue = sortDoc.getSortValue(this.field); if (sortValue != null) { if (sortValue.isPresent()) { - float val = (float) sortValue.getCurrentValue(); - ew.put(this.field, val); - return true; + val = (float) sortValue.getCurrentValue(); } else { // empty-value return false; } @@ -51,12 +50,12 @@ public boolean write( docValuesCache.getNumericDocValues( sortDoc.docId, readerContext.reader(), readerContext.ord); if (vals != null) { - int val = (int) vals.longValue(); - ew.put(this.field, Float.intBitsToFloat(val)); - return true; + val = Float.intBitsToFloat((int) vals.longValue()); } else { return false; } } + ew.put(this.field, val); + return true; } } diff --git a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java index ce42467cda0..0b75914f27b 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java @@ -24,7 +24,7 @@ import org.apache.solr.search.DocValuesIteratorCache; class LongFieldWriter extends FieldWriter { - private final String field; + protected final String field; private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; @@ -56,7 +56,11 @@ public boolean write( return false; } } - ew.put(field, val); + doWrite(ew, val); return true; } + + protected void doWrite(MapWriter.EntryWriter ew, long val) throws IOException { + ew.put(field, val); + } } From da4c8b808f8fb41602b27990806b05f22d8cca1d Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Tue, 19 Sep 2023 16:49:01 -0400 Subject: [PATCH 4/8] account for completely empty fields in ExportWriter --- .../solr/handler/export/BoolFieldWriter.java | 7 ++- .../solr/handler/export/DateFieldWriter.java | 5 ++- .../handler/export/DoubleFieldWriter.java | 5 ++- .../solr/handler/export/ExportWriter.java | 44 ++++++++++++------- .../solr/handler/export/FloatFieldWriter.java | 5 ++- .../solr/handler/export/IntFieldWriter.java | 5 ++- .../solr/handler/export/LongFieldWriter.java | 5 ++- .../solr/handler/export/MultiFieldWriter.java | 4 +- .../handler/export/StringFieldWriter.java | 7 ++- 9 files changed, 56 insertions(+), 31 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java index ea74856dcad..cf32497b5d7 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/BoolFieldWriter.java @@ -24,8 +24,11 @@ import org.apache.solr.search.DocValuesIteratorCache; class BoolFieldWriter extends StringFieldWriter { - public BoolFieldWriter(String field, FieldType fieldType, DocValuesIteratorCache dvIterCache) { - super(field, fieldType, dvIterCache); + public BoolFieldWriter( + String field, + FieldType fieldType, + DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { + super(field, fieldType, docValuesCache); } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java index b0b9704ffb2..7e32e98d5b8 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DateFieldWriter.java @@ -23,8 +23,9 @@ import org.apache.solr.search.DocValuesIteratorCache; class DateFieldWriter extends LongFieldWriter { - public DateFieldWriter(String field, DocValuesIteratorCache dvIterCache) { - super(field, dvIterCache); + public DateFieldWriter( + String field, DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { + super(field, docValuesCache); } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java index 2dc81cafbd4..e439560894b 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/DoubleFieldWriter.java @@ -27,9 +27,10 @@ class DoubleFieldWriter extends FieldWriter { private final String field; private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public DoubleFieldWriter(String field, DocValuesIteratorCache dvIterCache) { + public DoubleFieldWriter( + String field, DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; - docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java index 22a27983009..0f9a124df4b 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java @@ -503,47 +503,51 @@ public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher schemaField + " Must have useDocValuesAsStored='true' to be used with export writer"); } - if (fieldType instanceof IntValueFieldType) { + DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache = dvIterCache.getSupplier(field); + + if (docValuesCache == null) { + writers[i] = EMPTY_FIELD_WRITER; + } else if (fieldType instanceof IntValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, docValuesCache); } else { - writers[i] = new IntFieldWriter(field, dvIterCache); + writers[i] = new IntFieldWriter(field, docValuesCache); } } else if (fieldType instanceof LongValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, docValuesCache); } else { - writers[i] = new LongFieldWriter(field, dvIterCache); + writers[i] = new LongFieldWriter(field, docValuesCache); } } else if (fieldType instanceof FloatValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, docValuesCache); } else { - writers[i] = new FloatFieldWriter(field, dvIterCache); + writers[i] = new FloatFieldWriter(field, docValuesCache); } } else if (fieldType instanceof DoubleValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, docValuesCache); } else { - writers[i] = new DoubleFieldWriter(field, dvIterCache); + writers[i] = new DoubleFieldWriter(field, docValuesCache); } } else if (fieldType instanceof StrField || fieldType instanceof SortableTextField) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, docValuesCache); } else { - writers[i] = new StringFieldWriter(field, fieldType, dvIterCache); + writers[i] = new StringFieldWriter(field, fieldType, docValuesCache); } } else if (fieldType instanceof DateValueFieldType) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, false, docValuesCache); } else { - writers[i] = new DateFieldWriter(field, dvIterCache); + writers[i] = new DateFieldWriter(field, docValuesCache); } } else if (fieldType instanceof BoolField) { if (multiValued) { - writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, dvIterCache); + writers[i] = new MultiFieldWriter(field, fieldType, schemaField, true, docValuesCache); } else { - writers[i] = new BoolFieldWriter(field, fieldType, dvIterCache); + writers[i] = new BoolFieldWriter(field, fieldType, docValuesCache); } } else { throw new IOException( @@ -553,6 +557,16 @@ public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher return writers; } + private static final FieldWriter EMPTY_FIELD_WRITER = + new FieldWriter() { + @Override + public boolean write( + SortDoc sortDoc, LeafReaderContext readerContext, EntryWriter out, int fieldIndex) + throws IOException { + return false; + } + }; + SortDoc getSortDoc(SolrIndexSearcher searcher, SortField[] sortFields) throws IOException { SortValue[] sortValues = new SortValue[sortFields.length]; IndexSchema schema = searcher.getSchema(); diff --git a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java index 518b7391412..a60c14e6b0a 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/FloatFieldWriter.java @@ -27,9 +27,10 @@ class FloatFieldWriter extends FieldWriter { private final String field; private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public FloatFieldWriter(String field, DocValuesIteratorCache dvIterCache) { + public FloatFieldWriter( + String field, DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; - docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java index 0a2e107efe1..bf0396d4ab8 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/IntFieldWriter.java @@ -27,9 +27,10 @@ class IntFieldWriter extends FieldWriter { private final String field; private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public IntFieldWriter(String field, DocValuesIteratorCache dvIterCache) { + public IntFieldWriter( + String field, DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; - docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java index 0b75914f27b..7961549477c 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/LongFieldWriter.java @@ -28,9 +28,10 @@ class LongFieldWriter extends FieldWriter { private final DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache; - public LongFieldWriter(String field, DocValuesIteratorCache dvIterCache) { + public LongFieldWriter( + String field, DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; - docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java index d64fa5d0605..7f5bdee4899 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/MultiFieldWriter.java @@ -47,7 +47,7 @@ public MultiFieldWriter( FieldType fieldType, SchemaField schemaField, boolean numeric, - DocValuesIteratorCache dvIterCache) { + DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; this.fieldType = fieldType; this.schemaField = schemaField; @@ -57,7 +57,7 @@ public MultiFieldWriter( } else { bitsToValue = null; } - docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java b/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java index ffa30440108..2f8d0963e3a 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/StringFieldWriter.java @@ -49,10 +49,13 @@ public String toString() { } }; - public StringFieldWriter(String field, FieldType fieldType, DocValuesIteratorCache dvIterCache) { + public StringFieldWriter( + String field, + FieldType fieldType, + DocValuesIteratorCache.FieldDocValuesSupplier docValuesCache) { this.field = field; this.fieldType = fieldType; - this.docValuesCache = dvIterCache.getSupplier(field); + this.docValuesCache = docValuesCache; } @Override From 1fd92c90b0f7952144c06c1b81af82db2094d73d Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Wed, 20 Sep 2023 12:38:42 -0400 Subject: [PATCH 5/8] add test; fix bug --- .../solr/search/DocValuesIteratorCache.java | 1 + .../search/TestDocValuesIteratorCache.java | 139 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java diff --git a/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java index 8ab989100fb..6a7b21740b5 100644 --- a/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java +++ b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java @@ -196,6 +196,7 @@ private DocIdSetIterator getDocValues( // been returned at its current position, and has therefore not been consumed and // is thus eligible to be returned directly. (singleValued dv iterators are always // eligible to be returned directly, as they have no concept of being "consumed") + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; return dv; } else if (localId <= currentDoc) { if (localId >= noMatchSince[leafOrd]) { diff --git a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java new file mode 100644 index 00000000000..1964e7cf209 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.search; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Collection; +import java.util.Random; +import java.util.Set; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.core.SolrCore; +import org.apache.solr.index.NoMergePolicyFactory; +import org.apache.solr.util.EmbeddedSolrServerTestRule; +import org.apache.solr.util.RefCounted; +import org.apache.solr.util.SolrClientTestRule; +import org.junit.ClassRule; + +public class TestDocValuesIteratorCache extends SolrTestCaseJ4 { + + private static final int DOC_COUNT = 1000; + + @ClassRule + public static final SolrClientTestRule solrClientTestRule = + new EmbeddedSolrServerTestRule() { + @Override + protected void before() throws Throwable { + // must set NoMergePolicyFactory, because OrdinalMap building depends on the predictable + // existence of multiple segments; if the merge policy happens to combine into a single + // segment, no OrdinalMap will be built, throwing off our tests + systemSetPropertySolrTestsMergePolicyFactory(NoMergePolicyFactory.class.getName()); + startSolr(LuceneTestCase.createTempDir()); + } + + @Override + protected void after() { + systemClearPropertySolrTestsMergePolicyFactory(); + super.after(); + } + }; + + @SuppressWarnings("try") + public void test() throws Exception { + Path configSet = LuceneTestCase.createTempDir(); + SolrTestCaseJ4.copyMinConf(configSet.toFile()); + Files.copy( + TEST_PATH().resolve("collection1/conf/schema-docValuesMulti.xml"), + configSet.resolve("conf/schema.xml"), + StandardCopyOption.REPLACE_EXISTING); + + solrClientTestRule.newCollection().withConfigSet(configSet.toString()).create(); + + SolrClient client = solrClientTestRule.getSolrClient(); + + Random r = random(); + String[][] expectVals = indexDocs(client, r); + + try (SolrCore core = + ((EmbeddedSolrServer) client).getCoreContainer().getCore(DEFAULT_TEST_CORENAME)) { + RefCounted sref = core.getSearcher(); + try (Closeable c = sref::decref) { + SolrIndexSearcher s = sref.get(); + assertEquals(DOC_COUNT, s.maxDoc()); + SolrDocumentFetcher docFetcher = s.getDocFetcher(); + DocValuesIteratorCache dvIterCache = new DocValuesIteratorCache(s); + final Set getFields = Set.of("stringdv"); + final SolrDocument doc = new SolrDocument(); + for (int i = DOC_COUNT * 10; i >= 0; i--) { + int checkId = r.nextInt(DOC_COUNT); + doc.clear(); + docFetcher.decorateDocValueFields(doc, checkId, getFields, dvIterCache); + String[] expected = expectVals[checkId]; + if (expected == null) { + assertTrue(doc.isEmpty()); + } else { + assertEquals(1, doc.size()); + Collection actualVals = doc.getFieldValues("stringdv"); + assertEquals(expected.length, actualVals.size()); + int j = 0; + for (Object o : actualVals) { + assertEquals(expected[j++], o); + } + } + } + } + } + } + + private String[][] indexDocs(SolrClient client, Random r) + throws SolrServerException, IOException { + String[][] ret = new String[DOC_COUNT][]; + int pct = r.nextInt(100); + for (int i = 0; i < DOC_COUNT; i++) { + if (r.nextInt(100) > pct) { + client.add(sdoc("id", Integer.toString(i))); + } else { + String str1 = TestUtil.randomSimpleString(r); + String str2 = TestUtil.randomSimpleString(r); + client.add(sdoc("id", Integer.toString(i), "stringdv", str1, "stringdv", str2)); + int cmp = str1.compareTo(str2); + if (cmp == 0) { + ret[i] = new String[] {str1}; + } else if (cmp < 0) { + ret[i] = new String[] {str1, str2}; + } else { + ret[i] = new String[] {str2, str1}; + } + } + if (r.nextInt(DOC_COUNT / 5) == 0) { + // aim for 5 segments + client.commit(); + } + } + client.commit(); + return ret; + } +} From 6d39653b8e4dab5d13062eaed80bbc3263887ddb Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Thu, 21 Sep 2023 10:21:12 -0400 Subject: [PATCH 6/8] slightly separate logic for single/multiValued, update test to cover both --- .../solr/search/DocValuesIteratorCache.java | 27 ++++++++---- .../search/TestDocValuesIteratorCache.java | 43 +++++++++++++------ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java index 6a7b21740b5..eba8a731881 100644 --- a/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java +++ b/solr/core/src/java/org/apache/solr/search/DocValuesIteratorCache.java @@ -190,15 +190,24 @@ private DocIdSetIterator getDocValues( } else { dv = perLeaf[leafOrd]; int currentDoc = dv.docID(); - if (localId == currentDoc - && (singleValued || noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS)) { - // `noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS` means that `dv` has not - // been returned at its current position, and has therefore not been consumed and - // is thus eligible to be returned directly. (singleValued dv iterators are always - // eligible to be returned directly, as they have no concept of being "consumed") - noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; - return dv; - } else if (localId <= currentDoc) { + if (localId == currentDoc) { + if (singleValued) { + return dv; + } else if (noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS) { + // `noMatchSince[leafOrd] != DocIdSetIterator.NO_MORE_DOCS` means that `dv` has not + // been returned at its current position, and has therefore not been consumed and + // is thus eligible to be returned directly. (singleValued dv iterators are always + // eligible to be returned directly, as they have no concept of being "consumed") + + // NOTE: we must reset `noMatchSince[leafOrd]` here in order to prevent returning + // consumed docValues; even though this actually loses us possible skipping information, + // it's an edge case, and allows us to use `noMatchSince[leafOrd]` as a signal of + // whether we have consumed multivalued docValues. + noMatchSince[leafOrd] = DocIdSetIterator.NO_MORE_DOCS; + return dv; + } + } + if (localId <= currentDoc) { if (localId >= noMatchSince[leafOrd]) { // if the requested doc falls between the last requested doc and the current // position, then we know there's no match. diff --git a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java index 1964e7cf209..338be897c70 100644 --- a/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java +++ b/solr/core/src/test/org/apache/solr/search/TestDocValuesIteratorCache.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.Collection; import java.util.Random; import java.util.Set; @@ -61,14 +60,27 @@ protected void after() { } }; + private static String fieldConfig(String fieldName, boolean multivalued) { + return "\n"; + } + + private static final String SINGLE = "single"; + private static final String MULTI = "multi"; + @SuppressWarnings("try") public void test() throws Exception { Path configSet = LuceneTestCase.createTempDir(); SolrTestCaseJ4.copyMinConf(configSet.toFile()); - Files.copy( - TEST_PATH().resolve("collection1/conf/schema-docValuesMulti.xml"), - configSet.resolve("conf/schema.xml"), - StandardCopyOption.REPLACE_EXISTING); + Path schemaXml = configSet.resolve("conf/schema.xml"); + Files.writeString( + schemaXml, + Files.readString(schemaXml) + .replace( + "", fieldConfig(SINGLE, false) + fieldConfig(MULTI, true) + "")); solrClientTestRule.newCollection().withConfigSet(configSet.toString()).create(); @@ -85,7 +97,7 @@ public void test() throws Exception { assertEquals(DOC_COUNT, s.maxDoc()); SolrDocumentFetcher docFetcher = s.getDocFetcher(); DocValuesIteratorCache dvIterCache = new DocValuesIteratorCache(s); - final Set getFields = Set.of("stringdv"); + final Set getFields = Set.of(SINGLE, MULTI); final SolrDocument doc = new SolrDocument(); for (int i = DOC_COUNT * 10; i >= 0; i--) { int checkId = r.nextInt(DOC_COUNT); @@ -95,10 +107,12 @@ public void test() throws Exception { if (expected == null) { assertTrue(doc.isEmpty()); } else { - assertEquals(1, doc.size()); - Collection actualVals = doc.getFieldValues("stringdv"); - assertEquals(expected.length, actualVals.size()); - int j = 0; + assertEquals(2, doc.size()); + Object singleValue = doc.getFieldValue(SINGLE); + Collection actualVals = doc.getFieldValues(MULTI); + assertEquals(expected.length, actualVals.size() + 1); // +1 for single-valued field + assertEquals(expected[0], singleValue); + int j = 1; for (Object o : actualVals) { assertEquals(expected[j++], o); } @@ -116,16 +130,17 @@ private String[][] indexDocs(SolrClient client, Random r) if (r.nextInt(100) > pct) { client.add(sdoc("id", Integer.toString(i))); } else { + String str = TestUtil.randomSimpleString(r); String str1 = TestUtil.randomSimpleString(r); String str2 = TestUtil.randomSimpleString(r); - client.add(sdoc("id", Integer.toString(i), "stringdv", str1, "stringdv", str2)); + client.add(sdoc("id", Integer.toString(i), SINGLE, str, MULTI, str1, MULTI, str2)); int cmp = str1.compareTo(str2); if (cmp == 0) { - ret[i] = new String[] {str1}; + ret[i] = new String[] {str, str1}; } else if (cmp < 0) { - ret[i] = new String[] {str1, str2}; + ret[i] = new String[] {str, str1, str2}; } else { - ret[i] = new String[] {str2, str1}; + ret[i] = new String[] {str, str2, str1}; } } if (r.nextInt(DOC_COUNT / 5) == 0) { From cec160588b2d523e665139e336d9bc031489d159 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Mon, 25 Sep 2023 08:36:10 -0400 Subject: [PATCH 7/8] move static field declaration to top of ExportWriter class --- .../solr/handler/export/ExportWriter.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java index 0f9a124df4b..51ba5551b69 100644 --- a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java +++ b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java @@ -100,6 +100,14 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable { public static final int DEFAULT_BATCH_SIZE = 30000; public static final int DEFAULT_QUEUE_SIZE = 150000; + private static final FieldWriter EMPTY_FIELD_WRITER = + new FieldWriter() { + @Override + public boolean write( + SortDoc sortDoc, LeafReaderContext readerContext, EntryWriter out, int fieldIndex) { + return false; + } + }; private OutputStreamWriter respWriter; final SolrQueryRequest req; @@ -557,16 +565,6 @@ public FieldWriter[] getFieldWriters(String[] fields, SolrIndexSearcher searcher return writers; } - private static final FieldWriter EMPTY_FIELD_WRITER = - new FieldWriter() { - @Override - public boolean write( - SortDoc sortDoc, LeafReaderContext readerContext, EntryWriter out, int fieldIndex) - throws IOException { - return false; - } - }; - SortDoc getSortDoc(SolrIndexSearcher searcher, SortField[] sortFields) throws IOException { SortValue[] sortValues = new SortValue[sortFields.length]; IndexSchema schema = searcher.getSchema(); From 8d11ef42e6e00058f256f40c93bed61b99981f65 Mon Sep 17 00:00:00 2001 From: Michael Gibney Date: Tue, 26 Sep 2023 10:11:54 -0400 Subject: [PATCH 8/8] add CHANGES.txt entry --- solr/CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ae656b4ad8f..e8382b835ff 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -149,6 +149,7 @@ Optimizations * SOLR-16265: reduce memory usage of ContentWriter based requests in Http2SolrClient (Alex Deparvu, Kevin Risden, David Smiley) +* SOLR-16989: Optimize and consolidate reuse of DocValues iterators for value retrieval (Michael Gibney) Bug Fixes ---------------------