This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 32bc2ee2da7 Prebuild Lucene doc ID mappings from numeric doc values
(#19897)
32bc2ee2da7 is described below
commit 32bc2ee2da7e91f29bc61d8154abe5b652cd7cda
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Oct 26 14:28:31 2026 -0700
Prebuild Lucene doc ID mappings from numeric doc values (#19897)
Build Lucene-to-Pinot mappings during immutable TEXT index creation,
persisting Skip for identity mappings and packaging reordered mappings. Retain
legacy stored-field fallback; new indexes require Pinot 1.4.0 or later readers.
---
.../MultiColumnRealtimeLuceneTextIndex.java | 2 +-
.../invertedindex/RealtimeLuceneTextIndex.java | 2 +-
.../creator/impl/text/LuceneTextIndexCreator.java | 112 ++++----
.../index/readers/text/LuceneTextIndexReader.java | 164 +++---------
.../text/MultiColumnLuceneTextIndexReader.java | 4 +-
.../local/segment/store/TextIndexUtils.java | 68 +++++
.../index/creator/LuceneTextIndexCreatorTest.java | 287 +++++++++++++++++++--
.../local/segment/index/loader/LoaderTest.java | 54 ++--
.../TextIndexRollbackRecoveryTest.java | 2 +-
.../text/LuceneTextIndexCompatibleTest.java | 174 ++++++++++++-
.../readers/text/LuceneTextIndexCountTest.java | 22 +-
.../segment/store/FilePerIndexDirectoryTest.java | 22 +-
.../store/SingleFileIndexDirectoryTest.java | 22 +-
.../pinot/segment/spi/index/TextIndexConfig.java | 6 +-
14 files changed, 675 insertions(+), 266 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java
index ba7f0a2f205..a7c84e31d93 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/MultiColumnRealtimeLuceneTextIndex.java
@@ -290,7 +290,7 @@ public class MultiColumnRealtimeLuceneTextIndex implements
MultiColumnTextIndexR
while (luceneDocIDIterator.hasNext()) {
int luceneDocId = luceneDocIDIterator.next();
Document document = indexSearcher.doc(luceneDocId);
- int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
+ int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
actualDocIDs.add(pinotDocId);
}
} catch (Exception e) {
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java
index 9fe514de892..8033139dbf6 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeLuceneTextIndex.java
@@ -258,7 +258,7 @@ public class RealtimeLuceneTextIndex implements
MutableTextIndex {
while (luceneDocIDIterator.hasNext()) {
int luceneDocId = luceneDocIDIterator.next();
Document document = indexSearcher.doc(luceneDocId);
- int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
+ int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
actualDocIDs.add(pinotDocId);
}
} catch (Exception e) {
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java
index 359e41d31d7..a144886426f 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java
@@ -29,6 +29,7 @@ import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.CharArraySet;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
+import org.apache.lucene.document.NumericDocValuesField;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.document.TextField;
import org.apache.lucene.index.DirectoryReader;
@@ -36,16 +37,17 @@ import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.index.LogByteSizeMergePolicy;
-import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
import org.apache.lucene.store.NRTCachingDirectory;
import
org.apache.pinot.segment.local.realtime.impl.invertedindex.LuceneNRTCachingMergePolicy;
import
org.apache.pinot.segment.local.segment.index.text.AbstractTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
import org.apache.pinot.segment.spi.V1Constants;
import org.apache.pinot.segment.spi.creator.IndexCreationContext;
import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.apache.pinot.segment.spi.index.TextIndexConfig.DocIdTranslatorMode;
import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
import org.slf4j.Logger;
@@ -57,7 +59,13 @@ import org.slf4j.LoggerFactory;
/// and realtime from
[org.apache.pinot.segment.local.realtime.impl.invertedindex.RealtimeLuceneTextIndex]
public class LuceneTextIndexCreator extends AbstractTextIndexCreator {
private static final Logger LOGGER =
LoggerFactory.getLogger(LuceneTextIndexCreator.class);
- public static final String LUCENE_INDEX_DOC_ID_COLUMN_NAME = "DocID";
+ /// Stored Pinot document ID used by mutable and legacy immutable indexes.
+ /// Also the alternate numeric doc-values field when the TEXT column is
named "PinotDocID".
+ public static final String PINOT_DOC_ID_STORED_FIELD_NAME = "DocID";
+
+ /// Preferred numeric doc-values field holding the Pinot document ID in
freshly built immutable indexes.
+ /// Used to build the Lucene-to-Pinot mapping; omitted when explicitly
configured with Skip.
+ public static final String PINOT_DOC_ID_DOC_VALUES_FIELD_NAME = "PinotDocID";
private final String _textColumn;
private final boolean _commitOnClose;
@@ -89,7 +97,7 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
/// been added), false if index should not be committed
/// @param realtimeConversion index creator should create an index using the
realtime segment
/// @param consumerDir consumer dir containing the realtime index, used when
realtimeConversion and commit is true
- /// @param immutableToMutableIdMap immutableToMutableIdMap from segment
conversion
+ /// @param mutableToImmutableDocIdMap mutableToImmutableDocIdMap from
segment conversion
/// @param combineAndCleanupFiles true if files should be combined and text
index directory should be removed
/// Note on commit:
/// Once
[org.apache.pinot.segment.local.segment.creator.impl.SegmentColumnarIndexCreator]
@@ -108,7 +116,7 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
/// to offline), we close this lucene index writer to release
resources but don't commit.
/// @param config the text index config
public LuceneTextIndexCreator(String column, File segmentIndexDir, boolean
commit, boolean realtimeConversion,
- @Nullable File consumerDir, boolean mutableSegmentCompacted, @Nullable
int[] immutableToMutableIdMap,
+ @Nullable File consumerDir, boolean mutableSegmentCompacted, @Nullable
int[] mutableToImmutableDocIdMap,
boolean combineAndCleanupFiles, TextIndexConfig config) {
_textColumn = column;
_commitOnClose = commit;
@@ -149,7 +157,7 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
if (_reuseMutableIndex) {
LOGGER.info("Reusing the realtime lucene index for segment {} and
column {}", segmentIndexDir, column);
indexWriterConfig.setOpenMode(IndexWriterConfig.OpenMode.CREATE_OR_APPEND);
- convertMutableSegment(segmentIndexDir, consumerDir,
immutableToMutableIdMap, indexWriterConfig);
+ convertMutableSegment(segmentIndexDir, consumerDir,
mutableToImmutableDocIdMap, indexWriterConfig);
return;
}
@@ -189,11 +197,11 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
/// @param commit true if the index should be committed
/// @param realtimeConversion index creator should create an index using the
realtime segment
/// @param consumerDir consumer dir containing the realtime index
- /// @param immutableToMutableIdMap immutableToMutableIdMap from segment
conversion
+ /// @param mutableToImmutableDocIdMap mutableToImmutableDocIdMap from
segment conversion
/// @param config the text index config
public LuceneTextIndexCreator(String column, File segmentIndexDir, boolean
commit, boolean realtimeConversion,
- @Nullable File consumerDir, @Nullable int[] immutableToMutableIdMap,
TextIndexConfig config) {
- this(column, segmentIndexDir, commit, realtimeConversion, consumerDir,
false, immutableToMutableIdMap, false,
+ @Nullable File consumerDir, @Nullable int[] mutableToImmutableDocIdMap,
TextIndexConfig config) {
+ this(column, segmentIndexDir, commit, realtimeConversion, consumerDir,
false, mutableToImmutableDocIdMap, false,
config);
}
@@ -237,15 +245,15 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
indexWriter.commit();
buildMappingFile(segmentIndexDir, _textColumn, destDirectory,
mutableToImmutableDocIdMap);
} catch (Exception e) {
- throw new RuntimeException("Failed to build the mapping file during
segment conversion: " + e);
+ throw new RuntimeException("Failed to build the mapping file during
segment conversion", e);
}
} catch (IOException e) {
throw new RuntimeException("Failed to convert the mutable lucene index:
" + e);
}
}
- /// Generate the mapping file from mutable Pinot docId (stored within the
Lucene index) to immutable Pinot docId using
- /// the mutableToImmutableDocIdMap from segment conversion
+ /// Build a mapping from final Lucene ids to Pinot ids, applying
mutable-to-immutable remapping during conversion.
+ /// New immutable indexes persist either the mapping or a verified Skip
setting for Pinot 1.4.0 and later readers.
/// @param segmentIndexDir segment index directory
/// @param column column name
/// @param directory directory of the index
@@ -253,39 +261,31 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
private void buildMappingFile(File segmentIndexDir, String column, Directory
directory,
@Nullable int[] mutableToImmutableDocIdMap)
throws IOException {
- IndexReader indexReader = DirectoryReader.open(directory);
- IndexSearcher indexSearcher = new IndexSearcher(indexReader);
-
- int numDocs = indexSearcher.getIndexReader().numDocs();
- int length = Integer.BYTES * numDocs;
- File docIdMappingFile = new
File(SegmentDirectoryPaths.findSegmentDirectory(segmentIndexDir),
- column +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- String desc = "Text index docId mapping buffer: " + column;
- try (PinotDataBuffer buffer = PinotDataBuffer.mapFile(docIdMappingFile, /*
readOnly */ false, 0, length,
- ByteOrder.LITTLE_ENDIAN, desc)) {
- try {
- // If immutableToMutableIdMap is null, then docIds should not change
between the mutable and immutable segments.
- // Therefore, the mapping file can be built without doing an
additional docId conversion
- if (mutableToImmutableDocIdMap == null) {
- for (int i = 0; i < numDocs; i++) {
- Document document = indexSearcher.storedFields().document(i);
- int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
- } else {
- for (int i = 0; i < numDocs; i++) {
- Document document = indexSearcher.storedFields().document(i);
- int mutableDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
- int immutableDocId = mutableToImmutableDocIdMap[mutableDocId];
- buffer.putInt(i * Integer.BYTES, immutableDocId);
- }
- }
+ if (_config.getDocIdTranslatorMode() == DocIdTranslatorMode.Skip) {
+ return;
+ }
+ try (IndexReader indexReader = DirectoryReader.open(directory)) {
+ int numDocs = indexReader.numDocs();
+ int length = Integer.BYTES * numDocs;
+ File docIdMappingFile = new
File(SegmentDirectoryPaths.findSegmentDirectory(segmentIndexDir),
+ column +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
+ String desc = "Text index docId mapping buffer: " + column;
+ boolean isIdentityMapping;
+ try (PinotDataBuffer buffer = PinotDataBuffer.mapFile(docIdMappingFile,
/* readOnly */ false, 0, length,
+ ByteOrder.LITTLE_ENDIAN, desc)) {
+ isIdentityMapping = TextIndexUtils.fillDocIdMapping(indexReader,
buffer, numDocs, mutableToImmutableDocIdMap);
} catch (Exception e) {
throw new RuntimeException(
"Caught exception while building mutable to immutable doc id
mapping for text index column: " + column, e);
}
- } finally {
- indexReader.close();
+ if (!_reuseMutableIndex || _config.getDocIdTranslatorMode() ==
DocIdTranslatorMode.TryOptimize) {
+ if (isIdentityMapping) {
+ FileUtils.delete(docIdMappingFile);
+ }
+ DocIdTranslatorMode mode = isIdentityMapping ?
DocIdTranslatorMode.Skip : DocIdTranslatorMode.Default;
+ TextIndexConfig newConfig = new
TextIndexConfigBuilder(_config).withDocIdTranslatorMode(mode.name()).build();
+ TextIndexUtils.writeConfigToPropertiesFile(_indexFile, newConfig);
+ }
}
}
@@ -298,7 +298,7 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
// text index on SV column
Document docToIndex = new Document();
docToIndex.add(new TextField(_textColumn, document, Field.Store.NO));
- docToIndex.add(new StoredField(LUCENE_INDEX_DOC_ID_COLUMN_NAME,
_nextDocId++));
+ addDocIdFields(docToIndex);
try {
_indexWriter.addDocument(docToIndex);
} catch (Exception e) {
@@ -321,7 +321,7 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
for (int i = 0; i < length; i++) {
docToIndex.add(new TextField(_textColumn, documents[i], Field.Store.NO));
}
- docToIndex.add(new StoredField(LUCENE_INDEX_DOC_ID_COLUMN_NAME,
_nextDocId++));
+ addDocIdFields(docToIndex);
try {
_indexWriter.addDocument(docToIndex);
@@ -331,6 +331,20 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
}
}
+ private void addDocIdFields(Document document) {
+ int docId = _nextDocId++;
+ if (!_commitOnClose) {
+ // Mutable indexes retain the stored field for existing readers and
segment conversion.
+ document.add(new StoredField(PINOT_DOC_ID_STORED_FIELD_NAME, docId));
+ } else if (_config.getDocIdTranslatorMode() != DocIdTranslatorMode.Skip) {
+ // Avoid sharing the numeric field with the TEXT column, including empty
multi-value documents.
+ String fieldName = _textColumn.equals(PINOT_DOC_ID_DOC_VALUES_FIELD_NAME)
+ ? PINOT_DOC_ID_STORED_FIELD_NAME
+ : PINOT_DOC_ID_DOC_VALUES_FIELD_NAME;
+ document.add(new NumericDocValuesField(fieldName, docId));
+ }
+ }
+
@Override
public void seal() {
if (_reuseMutableIndex) {
@@ -408,17 +422,9 @@ public class LuceneTextIndexCreator extends
AbstractTextIndexCreator {
try {
// based on the commit flag set in IndexWriterConfig, this will decide
to commit or not
_indexWriter.close();
- // Build docIdMapping file if storeInSegmentFile is true
- // This allows the mapping file to be available during read without
building it on-the-fly
- if (_config.isStoreInSegmentFile()) {
- //Check if mapping file already exists
- File mappingFile = new
File(SegmentDirectoryPaths.findSegmentDirectory(_segmentDirectory),
- _textColumn +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- if (!mappingFile.exists()) {
- LOGGER.info("lucene doc IdMapping file doesn't exists for column:
{}, building mapping file", _textColumn);
- // Build the docId mapping file so it's available during segment load
- buildMappingFile(_segmentDirectory, _textColumn, _indexDirectory,
null);
- }
+ if (_commitOnClose) {
+ // Older readers require the final mapping or verified Skip setting
when the stored DocID is absent.
+ buildMappingFile(_segmentDirectory, _textColumn, _indexDirectory,
null);
}
_indexDirectory.close();
} catch (Exception e) {
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
index 6e73e11f10d..77e79cabafb 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java
@@ -29,13 +29,8 @@ import java.util.Map;
import java.util.Properties;
import javax.annotation.Nullable;
import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.document.Document;
-import org.apache.lucene.document.DocumentStoredFieldVisitor;
import org.apache.lucene.index.DirectoryReader;
-import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.index.IndexableField;
-import org.apache.lucene.index.StoredFields;
import org.apache.lucene.queryparser.classic.QueryParser;
import org.apache.lucene.queryparser.classic.QueryParserBase;
import org.apache.lucene.search.Collector;
@@ -43,7 +38,6 @@ import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
-import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
import org.apache.pinot.segment.local.utils.LuceneTextIndexUtils;
@@ -242,6 +236,7 @@ public class LuceneTextIndexReader implements
TextIndexReader {
MutableRoaringBitmap docIds = new MutableRoaringBitmap();
Collector docIDCollector = new LuceneDocIdCollector(docIds,
_docIdTranslator);
try {
+ validateDocCount();
_indexSearcher.search(buildQuery(searchQuery, optionsString,
queryScopedCache), docIDCollector);
return docIds;
} catch (QueryException e) {
@@ -257,6 +252,15 @@ public class LuceneTextIndexReader implements
TextIndexReader {
}
}
+ private void validateDocCount() {
+ int maxDoc = _indexReader.maxDoc();
+ // Collectors address maxDoc() space, including deleted documents. Skip
mode has no mapping-buffer bounds check.
+ if (maxDoc > _numDocs) {
+ throw new IllegalStateException(
+ "Lucene text index contains " + maxDoc + " documents, exceeding
segment document count: " + _numDocs);
+ }
+ }
+
/// Identifies a parsing configuration. A record so that equality is
structural: comparing the
/// rendered text of these lists instead would conflate `["failed",
"order"]` with
/// `["failed, order"]`, which are different stop-word sets that parse the
same string differently.
@@ -328,15 +332,7 @@ public class LuceneTextIndexReader implements
TextIndexReader {
public int getNumMatchingDocs(String searchQuery, @Nullable String
optionsString,
@Nullable QueryScopedCache queryScopedCache) {
try {
- // A Lucene index larger than the segment means the doc-id mapping is
broken -- a state getDocIds
- // surfaces as an IndexOutOfBoundsException from the translator.
Counting does not consult the
- // translator, so without this guard the corruption would become a
silently inflated count instead.
- // Compared against maxDoc(), not numDocs(): LuceneDocIdCollector
indexes the translator by
- // `context.docBase + doc`, which is maxDoc() space, and with deletions
numDocs() < maxDoc() would let a
- // broken index slip through. Fall back so that it keeps failing loudly.
- if (_indexReader.maxDoc() > _numDocs) {
- return getDocIds(searchQuery, optionsString,
queryScopedCache).getCardinality();
- }
+ validateDocCount();
return LuceneTextIndexUtils.countWithoutMaterializing(_indexSearcher,
buildQuery(searchQuery, optionsString, queryScopedCache));
} catch (QueryException e) {
@@ -479,73 +475,35 @@ public class LuceneTextIndexReader implements
TextIndexReader {
PinotDataBuffer.mapFile(docIdMappingFile, /* readOnly */ true, 0,
length, ByteOrder.LITTLE_ENDIAN, desc);
return new DefaultDocIdTranslator(buffer);
- } else {
- buffer =
- PinotDataBuffer.mapFile(docIdMappingFile, /* readOnly */ false, 0,
length, ByteOrder.LITTLE_ENDIAN, desc);
-
- if (config.getDocIdTranslatorMode() ==
DocIdTranslatorMode.TryOptimize) {
- LOGGER.debug("Creating lucene to pinot doc id mapping.");
- boolean allIdsAreEqual = true;
-
- class DocIdVisitor extends DocumentStoredFieldVisitor {
- int _pinotDocId;
-
- @Override
- public Status needsField(FieldInfo fieldInfo) {
- // assume doc id is the only stored document
- assert
LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME.equals(fieldInfo.name);
- return Status.YES;
- }
-
- @Override
- public void intField(FieldInfo fieldInfo, int value) {
- _pinotDocId = value;
- }
- }
-
- DocIdVisitor visitor = new DocIdVisitor();
- StoredFields storedFields = indexSearcher.storedFields();
-
- for (int i = 0; i < numDocs; i++) {
- storedFields.document(i, visitor);
- int pinotDocId = visitor._pinotDocId;
- allIdsAreEqual &= (i == pinotDocId);
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
-
- if (allIdsAreEqual) {
- LOGGER.debug("Lucene doc ids are equal to Pinot's. Deleting
mapping and updating index settings.");
- // TODO: it'd be better to unmap without flushing the buffer. Only
some buffer types support it, though.
- buffer.close();
- buffer = null;
- // get rid of mapping and use lucene ids
- docIdMappingFile.delete();
-
- // mapping is unnecessary so store flag in config file to skip
checking on next load
- TextIndexConfig newConfig =
- new
TextIndexConfigBuilder(config).withDocIdTranslatorMode(DocIdTranslatorMode.Skip.name()).build();
- TextIndexUtils.writeConfigToPropertiesFile(indexDir, newConfig);
-
- return NoOpDocIdTranslator.INSTANCE;
- } else {
- LOGGER.debug("Lucene doc ids are not equal to Pinot's. Keeping the
mapping.");
- // mapping is required so switch to default mode
- TextIndexConfig newConfig =
- new
TextIndexConfigBuilder(config).withDocIdTranslatorMode(DocIdTranslatorMode.Default.name()).build();
- TextIndexUtils.writeConfigToPropertiesFile(indexDir, newConfig);
-
- return new DefaultDocIdTranslator(buffer);
- }
- } else {
- for (int i = 0; i < numDocs; i++) {
- Document document = indexSearcher.doc(i);
- IndexableField field =
document.getField(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME);
- int pinotDocId = Integer.parseInt(field.stringValue());
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
- return new DefaultDocIdTranslator(buffer);
+ }
+ buffer =
+ PinotDataBuffer.mapFile(docIdMappingFile, /* readOnly */ false, 0,
length, ByteOrder.LITTLE_ENDIAN, desc);
+ LOGGER.debug("Creating lucene to pinot doc id mapping.");
+ boolean isIdentityMapping =
+ TextIndexUtils.fillDocIdMapping(indexSearcher.getIndexReader(),
buffer, numDocs, null);
+ if (config.getDocIdTranslatorMode() == DocIdTranslatorMode.TryOptimize) {
+ if (isIdentityMapping) {
+ LOGGER.debug("Lucene doc ids are equal to Pinot's. Deleting mapping
and updating index settings.");
+ // TODO: it'd be better to unmap without flushing the buffer. Only
some buffer types support it, though.
+ buffer.close();
+ buffer = null;
+ // get rid of mapping and use lucene ids
+ docIdMappingFile.delete();
+
+ // mapping is unnecessary so store flag in config file to skip
checking on next load
+ TextIndexConfig newConfig =
+ new
TextIndexConfigBuilder(config).withDocIdTranslatorMode(DocIdTranslatorMode.Skip.name()).build();
+ TextIndexUtils.writeConfigToPropertiesFile(indexDir, newConfig);
+
+ return NoOpDocIdTranslator.INSTANCE;
}
+ LOGGER.debug("Lucene doc ids are not equal to Pinot's. Keeping the
mapping.");
+ // mapping is required so switch to default mode
+ TextIndexConfig newConfig =
+ new
TextIndexConfigBuilder(config).withDocIdTranslatorMode(DocIdTranslatorMode.Default.name()).build();
+ TextIndexUtils.writeConfigToPropertiesFile(indexDir, newConfig);
}
+ return new DefaultDocIdTranslator(buffer);
} catch (Exception e) {
if (buffer != null) {
buffer.close();
@@ -579,53 +537,17 @@ public class LuceneTextIndexReader implements
TextIndexReader {
PinotDataBuffer buffer = PinotDataBuffer.allocateDirect(length,
ByteOrder.LITTLE_ENDIAN, desc);
try {
+ LOGGER.debug("Creating lucene to pinot doc id mapping.");
+ boolean isIdentityMapping =
TextIndexUtils.fillDocIdMapping(_indexReader, buffer, numDocs, null);
if (config.getDocIdTranslatorMode() == DocIdTranslatorMode.TryOptimize) {
- LOGGER.debug("Creating lucene to pinot doc id mapping.");
- boolean allIdsAreEqual = true;
-
- class DocIdVisitor extends DocumentStoredFieldVisitor {
- int _pinotDocId;
-
- @Override
- public Status needsField(FieldInfo fieldInfo) {
- // assume doc id is the only stored document
- assert
LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME.equals(fieldInfo.name);
- return Status.YES;
- }
-
- @Override
- public void intField(FieldInfo fieldInfo, int value) {
- _pinotDocId = value;
- }
- }
-
- DocIdVisitor visitor = new DocIdVisitor();
- StoredFields storedFields = _indexSearcher.storedFields();
-
- for (int i = 0; i < numDocs; i++) {
- storedFields.document(i, visitor);
- int pinotDocId = visitor._pinotDocId;
- allIdsAreEqual &= (i == pinotDocId);
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
-
- if (allIdsAreEqual) {
+ if (isIdentityMapping) {
LOGGER.debug("Lucene doc ids are equal to Pinot's. Using no-op
translator.");
buffer.close();
return NoOpDocIdTranslator.INSTANCE;
- } else {
- LOGGER.debug("Lucene doc ids are not equal to Pinot's. Using default
translator.");
- return new DefaultDocIdTranslator(buffer);
}
- } else {
- for (int i = 0; i < numDocs; i++) {
- Document document = _indexSearcher.doc(i);
- IndexableField field =
document.getField(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME);
- int pinotDocId = Integer.parseInt(field.stringValue());
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
- return new DefaultDocIdTranslator(buffer);
+ LOGGER.debug("Lucene doc ids are not equal to Pinot's. Using default
translator.");
}
+ return new DefaultDocIdTranslator(buffer);
} catch (Exception e) {
buffer.close();
throw new RuntimeException("Caught exception while creating doc id
translator for text index column: " + _column,
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/MultiColumnLuceneTextIndexReader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/MultiColumnLuceneTextIndexReader.java
index 9388798250f..adeba3b9a3c 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/MultiColumnLuceneTextIndexReader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/MultiColumnLuceneTextIndexReader.java
@@ -475,7 +475,7 @@ public class MultiColumnLuceneTextIndexReader implements
MultiColumnTextIndexRea
@Override
public Status needsField(FieldInfo fieldInfo) {
// assume doc id is the only stored document
- assert
LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME.equals(fieldInfo.name);
+ assert
LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME.equals(fieldInfo.name);
return Status.YES;
}
@@ -521,7 +521,7 @@ public class MultiColumnLuceneTextIndexReader implements
MultiColumnTextIndexRea
} else {
for (int i = 0; i < numDocs; i++) {
Document document = indexSearcher.storedFields().document(i);
- IndexableField field =
document.getField(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME);
+ IndexableField field =
document.getField(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME);
int pinotDocId = Integer.parseInt(field.stringValue());
buffer.putInt(i * Integer.BYTES, pinotDocId);
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
index 3f97bc15618..e7addf0e5f0 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
@@ -36,6 +36,12 @@ import org.apache.commons.io.FileUtils;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.CharArraySet;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.MultiDocValues;
+import org.apache.lucene.index.NumericDocValues;
+import org.apache.lucene.index.StoredFieldVisitor;
+import org.apache.lucene.index.StoredFields;
import org.apache.lucene.queryparser.classic.QueryParserBase;
import org.apache.lucene.search.IndexSearcher;
import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
@@ -89,6 +95,68 @@ public class TextIndexUtils {
}
}
+ /// Reads the actual Pinot ids after Lucene merges and sorting, optionally
remapping mutable ids during conversion.
+ /// Doc values avoid stored-field decompression. Mutable indexes contain
only the stored field; older immutable
+ /// indexes and documents without doc values also use the per-document
stored-field fallback.
+ /// Returns whether the final mapping is the identity mapping.
+ public static boolean fillDocIdMapping(IndexReader indexReader,
PinotDataBuffer buffer, int numDocs,
+ @Nullable int[] mutableToImmutableDocIdMap)
+ throws IOException {
+ NumericDocValues pinotDocIdValues =
+ MultiDocValues.getNumericValues(indexReader,
LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME);
+ if (pinotDocIdValues == null) {
+ // A TEXT column named PinotDocID uses the alternate numeric field to
avoid a field-schema collision.
+ pinotDocIdValues =
+ MultiDocValues.getNumericValues(indexReader,
LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME);
+ }
+ PinotDocIdVisitor visitor = new PinotDocIdVisitor();
+ StoredFields storedFields = null;
+ boolean isIdentityMapping = true;
+ for (int luceneDocId = 0; luceneDocId < numDocs; luceneDocId++) {
+ int pinotDocId;
+ if (pinotDocIdValues != null &&
pinotDocIdValues.advanceExact(luceneDocId)) {
+ pinotDocId = Math.toIntExact(pinotDocIdValues.longValue());
+ } else {
+ if (storedFields == null) {
+ storedFields = indexReader.storedFields();
+ }
+ visitor._pinotDocId = Integer.MIN_VALUE;
+ storedFields.document(luceneDocId, visitor);
+ pinotDocId = visitor._pinotDocId;
+ if (pinotDocId == Integer.MIN_VALUE) {
+ throw new IOException("Missing Pinot document id for Lucene
document: " + luceneDocId);
+ }
+ }
+ if (mutableToImmutableDocIdMap != null) {
+ // Conversion can reorder Pinot rows independently of Lucene's own
document order.
+ pinotDocId = mutableToImmutableDocIdMap[pinotDocId];
+ }
+ isIdentityMapping &= luceneDocId == pinotDocId;
+ buffer.putInt((long) luceneDocId * Integer.BYTES, pinotDocId);
+ }
+ return isIdentityMapping;
+ }
+
+ /// Visits only the stored Pinot document id. Each mapping operation owns
its visitor.
+ private static class PinotDocIdVisitor extends StoredFieldVisitor {
+ private int _pinotDocId;
+
+ @Override
+ public Status needsField(FieldInfo fieldInfo) {
+ return
LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME.equals(fieldInfo.name) ?
Status.YES : Status.NO;
+ }
+
+ @Override
+ public void intField(FieldInfo fieldInfo, int value) {
+ _pinotDocId = value;
+ }
+
+ @Override
+ public void stringField(FieldInfo fieldInfo, String value) {
+ _pinotDocId = Integer.parseInt(value);
+ }
+ }
+
public static void cleanupTextIndex(File segDir, String column) {
// Remove the lucene index file and potentially the docId mapping file.
File luceneIndexFile = new File(segDir, column +
Indexes.LUCENE_TEXT_INDEX_FILE_EXTENSION);
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/LuceneTextIndexCreatorTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/LuceneTextIndexCreatorTest.java
index 04dbb8a71a9..e6b2cc754ee 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/LuceneTextIndexCreatorTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/LuceneTextIndexCreatorTest.java
@@ -20,6 +20,7 @@ package org.apache.pinot.segment.local.segment.index.creator;
import java.io.File;
import java.io.IOException;
+import java.nio.ByteOrder;
import java.nio.file.Files;
import java.util.HashMap;
import org.apache.commons.io.FileUtils;
@@ -31,18 +32,33 @@ import org.apache.lucene.document.TextField;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.MultiDocValues;
+import org.apache.lucene.index.NumericDocValues;
+import org.apache.lucene.index.Term;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexBufferReader;
import
org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexReader;
import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
import org.apache.pinot.segment.spi.V1Constants;
import org.apache.pinot.segment.spi.index.TextIndexConfig;
-import org.testng.Assert;
+import org.apache.pinot.segment.spi.index.TextIndexConfig.DocIdTranslatorMode;
+import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
+import org.apache.pinot.spi.config.table.FieldConfig;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+
public class LuceneTextIndexCreatorTest {
private static final File INDEX_DIR =
@@ -77,9 +93,9 @@ public class LuceneTextIndexCreatorTest {
// Use TextIndex reader to validate that reads work
try (LuceneTextIndexReader reader = new LuceneTextIndexReader("foo",
INDEX_DIR, 5, new HashMap<>())) {
int[] matchedDocIds = reader.getDocIds("clean").toArray();
- Assert.assertEquals(matchedDocIds.length, 2);
- Assert.assertEquals(matchedDocIds[0], 0);
- Assert.assertEquals(matchedDocIds[1], 4);
+ assertEquals(matchedDocIds.length, 2);
+ assertEquals(matchedDocIds[0], 0);
+ assertEquals(matchedDocIds[1], 4);
}
}
@@ -89,8 +105,8 @@ public class LuceneTextIndexCreatorTest {
// Use TextIndex reader to validate that reads work
try (LuceneTextIndexReader reader = new LuceneTextIndexReader("foo",
INDEX_DIR, 5, new HashMap<>())) {
int[] matchedDocIds = reader.getDocIds("hold").toArray();
- Assert.assertEquals(matchedDocIds.length, 1);
- Assert.assertEquals(matchedDocIds[0], 3);
+ assertEquals(matchedDocIds.length, 1);
+ assertEquals(matchedDocIds[0], 3);
}
}
@@ -100,8 +116,8 @@ public class LuceneTextIndexCreatorTest {
// Use TextIndex reader to validate that reads work
try (LuceneTextIndexReader reader = new LuceneTextIndexReader("foo",
INDEX_DIR, 5, new HashMap<>())) {
int[] matchedDocIds = reader.getDocIds("retain").toArray();
- Assert.assertEquals(matchedDocIds.length, 1);
- Assert.assertEquals(matchedDocIds[0], 1);
+ assertEquals(matchedDocIds.length, 1);
+ assertEquals(matchedDocIds[0], 1);
}
}
@@ -111,16 +127,185 @@ public class LuceneTextIndexCreatorTest {
// Use TextIndex reader to validate that reads work
try (LuceneTextIndexReader reader = new LuceneTextIndexReader("foo",
INDEX_DIR, 5, new HashMap<>())) {
int[] matchedDocIds = reader.getDocIds("retain|keep").toArray();
- Assert.assertEquals(matchedDocIds.length, 2);
- Assert.assertEquals(matchedDocIds[0], 1);
- Assert.assertEquals(matchedDocIds[1], 2);
+ assertEquals(matchedDocIds.length, 2);
+ assertEquals(matchedDocIds[0], 1);
+ assertEquals(matchedDocIds[1], 2);
}
}
- /// Regression test for the AIOOB triggered when a prior segment-conversion
attempt left stale
- /// Lucene files at the v1 destination directory and the reuse-mutable-index
path is taken.
@Test
- public void testConvertMutableSegmentCleansStaleDestFiles()
+ public void testDocValuesReplaceStoredIdsForSingleAndMultiValueDocuments()
+ throws IOException {
+ String column = "values";
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator =
+ new LuceneTextIndexCreator(column, INDEX_DIR, true, false, null, null,
config)) {
+ creator.add("");
+ creator.add(new String[]{"alpha", "beta"}, 2);
+ creator.add(new String[0], 0);
+ creator.add("gamma");
+ creator.seal();
+ }
+
+ File indexDir = new File(INDEX_DIR, column +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
+ try (Directory directory = FSDirectory.open(indexDir.toPath());
+ DirectoryReader reader = DirectoryReader.open(directory)) {
+ NumericDocValues ids =
+ MultiDocValues.getNumericValues(reader,
LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME);
+ assertNull(MultiDocValues.getNumericValues(reader,
LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
+ assertEquals(reader.maxDoc(), 4);
+ for (int i = 0; i < reader.maxDoc(); i++) {
+ assertTrue(ids.advanceExact(i));
+ assertEquals(ids.longValue(), i);
+
assertNull(reader.storedFields().document(i).get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
+ }
+ }
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(column,
INDEX_DIR, 4, config)) {
+ assertEquals(reader.getDocIds("alpha", null).toArray(), new int[]{1});
+ assertEquals(reader.getDocIds("gamma", null).toArray(), new int[]{3});
+ assertEquals(reader.getDocIds("*:* ", null).toArray(), new int[]{0, 1,
2, 3});
+ }
+ }
+
+ @DataProvider
+ public Object[][] persistedMappingModes() {
+ return new Object[][]{
+ {false, true, null}, {true, true, DocIdTranslatorMode.Default},
+ {false, false, DocIdTranslatorMode.Default}, {true, false,
DocIdTranslatorMode.TryOptimize},
+ {false, true, DocIdTranslatorMode.Skip}, {true, true,
DocIdTranslatorMode.Skip}
+ };
+ }
+
+ @Test(dataProvider = "persistedMappingModes")
+ public void testPersistedIndexMapping(boolean combined, boolean identity,
DocIdTranslatorMode translatorMode)
+ throws Exception {
+ String column = "mapping";
+ TextIndexConfig config = new
TextIndexConfigBuilder().setStoreInSegmentFile(combined)
+ .withDocIdTranslatorMode(translatorMode == null ? null :
translatorMode.name()).build();
+ int[] pinotDocIds = identity ? new int[]{0, 1, 2} : new int[]{2, 0, 1};
+ try (LuceneTextIndexCreator creator =
+ new LuceneTextIndexCreator(column, INDEX_DIR, true, false, null,
false, null, combined, config)) {
+ creator.add("alpha");
+ creator.add("beta");
+ creator.add("gamma");
+ if (!identity) {
+ String[] terms = {"alpha", "beta", "gamma"};
+ for (int i = 0; i < terms.length; i++) {
+ creator.getIndexWriter().updateNumericDocValue(new Term(column,
terms[i]),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME,
pinotDocIds[i]);
+ }
+ }
+ creator.seal();
+ try (DirectoryReader reader =
DirectoryReader.open(creator.getIndexWriter())) {
+ NumericDocValues ids =
+ MultiDocValues.getNumericValues(reader,
LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME);
+ if (translatorMode == DocIdTranslatorMode.Skip) {
+ assertNull(ids);
+ } else {
+ assertNotNull(ids);
+ }
+ for (int i = 0; i < pinotDocIds.length; i++) {
+
assertNull(reader.storedFields().document(i).get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
+ if (ids != null) {
+ assertTrue(ids.advanceExact(i));
+ assertEquals(ids.longValue(), pinotDocIds[i]);
+ }
+ }
+ }
+ }
+
+ File indexDir = new File(INDEX_DIR, column +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
+ File combinedFile = new File(INDEX_DIR, column +
V1Constants.Indexes.LUCENE_COMBINE_TEXT_INDEX_FILE_EXTENSION);
+ String mappingFileName = column +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION;
+ try (PinotDataBuffer buffer = combined
+ ? PinotDataBuffer.mapFile(combinedFile, true, 0,
combinedFile.length(), ByteOrder.LITTLE_ENDIAN,
+ "combined text index test")
+ : null) {
+ assertEquals(combined
+ ? LuceneTextIndexBufferReader.hasFile(buffer, mappingFileName)
+ : new File(INDEX_DIR, mappingFileName).exists(), !identity);
+ DocIdTranslatorMode persistedMode = identity ? DocIdTranslatorMode.Skip
: DocIdTranslatorMode.Default;
+ if (combined) {
+ assertEquals(LuceneTextIndexBufferReader.extractProperties(buffer,
column)
+
.getProperty(FieldConfig.TEXT_INDEX_LUCENE_DOC_ID_TRANSLATOR_MODE),
persistedMode.name());
+ } else {
+ assertEquals(TextIndexUtils.getUpdatedConfigFromPropertiesFile(
+ new File(indexDir,
V1Constants.Indexes.LUCENE_TEXT_INDEX_PROPERTIES_FILE),
config).getDocIdTranslatorMode(),
+ persistedMode);
+ }
+ try (LuceneTextIndexReader reader = combined
+ ? new LuceneTextIndexReader(column, buffer, 3, config)
+ : new LuceneTextIndexReader(column, INDEX_DIR, 3, config)) {
+ assertEquals(reader.getDocIds("alpha", null).toArray(), new
int[]{pinotDocIds[0]});
+ assertEquals(reader.getDocIds("gamma", null).toArray(), new
int[]{pinotDocIds[2]});
+ assertEquals(reader.getDocIds("*:*", null).toArray(), new int[]{0, 1,
2});
+ }
+ }
+ }
+
+ @Test
+ public void testDocValuesFieldNameColumnWithEmptyValues()
+ throws IOException {
+ String column = LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME;
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator =
+ new LuceneTextIndexCreator(column, INDEX_DIR, true, false, null, null,
config)) {
+ creator.add(new String[0], 0);
+ creator.add(new String[]{"alpha"}, 1);
+ creator.seal();
+ try (DirectoryReader reader =
DirectoryReader.open(creator.getIndexWriter())) {
+ NumericDocValues ids =
+ MultiDocValues.getNumericValues(reader,
LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME);
+ for (int i = 0; i < 2; i++) {
+ assertTrue(ids.advanceExact(i));
+ assertEquals(ids.longValue(), i);
+
assertNull(reader.storedFields().document(i).get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME));
+ }
+ }
+ }
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(column,
INDEX_DIR, 2, config)) {
+ assertEquals(reader.getDocIds("alpha", null).toArray(), new int[]{1});
+ assertEquals(reader.getDocIds("*:* ", null).toArray(), new int[]{0, 1});
+ }
+ }
+
+ @Test
+ public void testMutableIndexRetainsStoredFieldFallback()
+ throws IOException {
+ String column = "mutableValues";
+ TextIndexConfig config = new TextIndexConfigBuilder().build();
+ try (LuceneTextIndexCreator creator =
+ new LuceneTextIndexCreator(column, INDEX_DIR, false, false, null,
null, config)) {
+ creator.add("alpha");
+ creator.add(new String[0], 0);
+ creator.getIndexWriter().commit();
+ try (DirectoryReader reader =
DirectoryReader.open(creator.getIndexWriter())) {
+ assertNull(MultiDocValues.getNumericValues(reader,
LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME));
+ assertEquals(reader.storedFields().document(0)
+ .get(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME), "0");
+ }
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(column,
INDEX_DIR, 2, config)) {
+ assertEquals(reader.getDocIds("alpha", null).toArray(), new int[]{0});
+ assertEquals(reader.getDocIds("*:* ", null).toArray(), new int[]{0,
1});
+ }
+ }
+ }
+
+ @DataProvider
+ public Object[][] mutableMappingModes() {
+ return new Object[][]{
+ {null, new int[]{0, 1, 2}, null, true},
+ {DocIdTranslatorMode.TryOptimize, new int[]{0, 1, 2}, null, false},
+ {DocIdTranslatorMode.TryOptimize, new int[]{0, 1, 2}, new int[]{2, 0,
1}, true},
+ // Only the final immutable IDs are sequential in this case.
+ {DocIdTranslatorMode.TryOptimize, new int[]{2, 0, 1}, new int[]{1, 2,
0}, false}
+ };
+ }
+
+ /// Regression test for stale destination files, including mappings made
identity or nonidentity by conversion.
+ @Test(dataProvider = "mutableMappingModes")
+ public void
testConvertMutableSegmentCleansStaleDestFiles(DocIdTranslatorMode
translatorMode, int[] mutableDocIds,
+ int[] mutableToImmutableDocIdMap, boolean mappingExpected)
throws Exception {
String column = "foo";
String segmentName = "regSeg";
@@ -135,7 +320,12 @@ public class LuceneTextIndexCreatorTest {
File mutableIndexDir = new File(new File(consumerDir, segmentName),
column + V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
FileUtils.forceMkdir(mutableIndexDir);
- writeLuceneDocs(mutableIndexDir, /* startDocId */ 0, numMutableDocs, /*
multiSegment */ false);
+ try (Directory directory = FSDirectory.open(mutableIndexDir.toPath());
+ IndexWriter writer = new IndexWriter(directory, new
IndexWriterConfig(new StandardAnalyzer()))) {
+ for (int mutableDocId : mutableDocIds) {
+ writer.addDocument(buildDoc(mutableDocId));
+ }
+ }
// 2. segmentIndexDir's parent
File tmpSegmentParent = new File(scratch, "tmp-" + segmentName + "-1");
@@ -154,16 +344,17 @@ public class LuceneTextIndexCreatorTest {
// 4. commit=true && realtimeConversion=true && reuseMutableIndex=true
triggers
// convertMutableSegment.
- TextIndexConfig config = new
TextIndexConfigBuilder().withReuseMutableIndex(true).build();
+ TextIndexConfig config = new
TextIndexConfigBuilder().withReuseMutableIndex(true)
+ .withDocIdTranslatorMode(translatorMode == null ? null :
translatorMode.name()).build();
try (LuceneTextIndexCreator creator = new LuceneTextIndexCreator(column,
segmentIndexDir,
/* commit */ true, /* realtimeConversion */ true, consumerDir,
- /* mutableSegmentCompacted */ false, /* immutableToMutableIdMap */
null,
+ /* mutableSegmentCompacted */ false, mutableToImmutableDocIdMap,
/* combineAndCleanupFiles */ false, config)) {
// Constructor performs the conversion; no add()/seal() on the reuse
path.
}
// 5a. Dest wipe: sentinel must be gone.
- Assert.assertFalse(staleSentinel.exists(),
+ assertFalse(staleSentinel.exists(),
"Stale file at v1 destination must be removed before conversion to "
+ "avoid CREATE_OR_APPEND picking up leftovers from a prior
attempt");
@@ -172,7 +363,7 @@ public class LuceneTextIndexCreatorTest {
// numStaleDocs instead.
try (Directory destDir = FSDirectory.open(destIndexDir.toPath());
DirectoryReader reader = DirectoryReader.open(destDir)) {
- Assert.assertEquals(reader.numDocs(), numMutableDocs,
+ assertEquals(reader.numDocs(), numMutableDocs,
"Converted Lucene index must contain exactly the mutable input's
document count, not "
+ "stale leftovers from a prior attempt");
}
@@ -181,24 +372,64 @@ public class LuceneTextIndexCreatorTest {
// segment's numDocs at read time - a mismatch here is what produced
AIOOB in production.
File mappingFile = new File(segmentIndexDir,
column +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- Assert.assertTrue(mappingFile.exists(), "DocId mapping file should be
built during conversion");
- Assert.assertEquals(mappingFile.length(), (long) Integer.BYTES *
numMutableDocs,
- "DocId mapping file must be sized for the segment's numDocs only");
+ assertEquals(mappingFile.exists(), mappingExpected);
+ if (mappingExpected) {
+ assertEquals(mappingFile.length(), (long) Integer.BYTES *
numMutableDocs,
+ "DocId mapping file must be sized for the segment's numDocs only");
+ }
+ if (translatorMode == DocIdTranslatorMode.TryOptimize) {
+ File propertiesFile = new File(destIndexDir,
V1Constants.Indexes.LUCENE_TEXT_INDEX_PROPERTIES_FILE);
+
assertEquals(TextIndexUtils.getUpdatedConfigFromPropertiesFile(propertiesFile,
config).getDocIdTranslatorMode(),
+ mappingExpected ? DocIdTranslatorMode.Default :
DocIdTranslatorMode.Skip);
+ }
// 5d. End-to-end read
try (LuceneTextIndexReader reader = new LuceneTextIndexReader(column,
segmentIndexDir,
numMutableDocs, config)) {
- int[] matched = reader.getDocIds("doc").toArray();
- for (int docId : matched) {
- Assert.assertTrue(docId >= 0 && docId < numMutableDocs,
- "Returned Pinot docId " + docId + " must be within [0, " +
numMutableDocs + ")");
- }
+ assertEquals(reader.getDocIds("doc", null).toArray(), new int[]{0, 1,
2});
+ assertEquals(reader.getDocIds("0", null).toArray(),
+ new int[]{mutableToImmutableDocIdMap == null ? 0 :
mutableToImmutableDocIdMap[0]});
}
} finally {
FileUtils.deleteDirectory(scratch);
}
}
+ @Test
+ public void testConvertMutableSegmentRejectsMissingStoredDocId()
+ throws IOException {
+ String column = "foo";
+ String segmentName = "missingDocId";
+ File consumerDir = new File(INDEX_DIR, "consumerDir");
+ File mutableIndexDir = new File(new File(consumerDir, segmentName),
+ column + V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
+ FileUtils.forceMkdir(mutableIndexDir);
+ try (Directory directory = FSDirectory.open(mutableIndexDir.toPath());
+ IndexWriter writer = new IndexWriter(directory, new
IndexWriterConfig(new StandardAnalyzer()))) {
+ writer.addDocument(buildDoc(0));
+ Document corruptDocument = buildDoc(1);
+
corruptDocument.removeField(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME);
+ writer.addDocument(corruptDocument);
+ writer.commit();
+ }
+
+ File segmentIndexDir = new File(new File(INDEX_DIR, "tmp-" + segmentName +
"-1"), segmentName);
+ FileUtils.forceMkdir(segmentIndexDir);
+ TextIndexConfig config = new
TextIndexConfigBuilder().withReuseMutableIndex(true).build();
+ RuntimeException exception = expectThrows(RuntimeException.class, () -> {
+ try (LuceneTextIndexCreator creator =
+ new LuceneTextIndexCreator(column, segmentIndexDir, true, true,
consumerDir, null, config)) {
+ // Conversion must reject the legacy index before a reader can consume
its mapping.
+ }
+ });
+ Throwable rootCause = exception;
+ while (rootCause.getCause() != null) {
+ rootCause = rootCause.getCause();
+ }
+ assertTrue(rootCause instanceof IOException);
+ assertEquals(rootCause.getMessage(), "Missing Pinot document id for Lucene
document: 1");
+ }
+
private static void writeLuceneDocs(File indexDir, int startDocId, int
count, boolean multiSegment)
throws IOException {
try (Directory directory = FSDirectory.open(indexDir.toPath());
@@ -224,7 +455,7 @@ public class LuceneTextIndexCreatorTest {
private static Document buildDoc(int docId) {
Document doc = new Document();
doc.add(new TextField("foo", "doc " + docId, Field.Store.NO));
- doc.add(new
StoredField(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME, docId));
+ doc.add(new
StoredField(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME, docId));
return doc;
}
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/LoaderTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/LoaderTest.java
index c61d4ad7fc6..ad08ef1f9e4 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/LoaderTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/LoaderTest.java
@@ -28,7 +28,9 @@ import
org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoa
import org.apache.pinot.segment.local.segment.creator.SegmentTestUtils;
import
org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
import
org.apache.pinot.segment.local.segment.index.converter.SegmentV1V2ToV3FormatConverter;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
import org.apache.pinot.segment.local.segment.store.SegmentLocalFSDirectory;
+import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
import org.apache.pinot.segment.local.utils.SegmentOperationsThrottler;
import org.apache.pinot.segment.local.utils.SegmentOperationsThrottlerSet;
import org.apache.pinot.segment.spi.ImmutableSegment;
@@ -39,6 +41,7 @@ import
org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
import org.apache.pinot.segment.spi.creator.SegmentVersion;
import org.apache.pinot.segment.spi.datasource.DataSource;
import org.apache.pinot.segment.spi.index.StandardIndexes;
+import org.apache.pinot.segment.spi.index.TextIndexConfig.DocIdTranslatorMode;
import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl;
import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
import org.apache.pinot.segment.spi.store.SegmentDirectory;
@@ -670,19 +673,12 @@ public class LoaderTest {
verifyIndexDirIsV3(_indexDir);
// no change/conversion should have happened for textIndex dir
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
- // segment load should have created the docID mapping file in V3 structure
- File textIndexDocIdMappingFile =
- SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
- assertNotNull(textIndexDocIdMappingFile);
assertTrue(textIndexFile.isDirectory());
- assertFalse(textIndexDocIdMappingFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
- assertEquals(textIndexDocIdMappingFile.getName(),
- TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- assertEquals(textIndexDocIdMappingFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ assertIdentityTextIndexMapping(indexSegment, textIndexFile);
indexSegment.destroy();
// CASE 2: set the segment version to load in IndexLoadingConfig as V3
@@ -699,18 +695,12 @@ public class LoaderTest {
verifyIndexDirIsV3(_indexDir);
// no change/conversion should have happened for textIndex dir
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
- // segment load should have created the docID mapping file in V3 structure
- textIndexDocIdMappingFile =
SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
- assertNotNull(textIndexDocIdMappingFile);
assertTrue(textIndexFile.isDirectory());
- assertFalse(textIndexDocIdMappingFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
- assertEquals(textIndexDocIdMappingFile.getName(),
- TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- assertEquals(textIndexDocIdMappingFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ assertIdentityTextIndexMapping(indexSegment, textIndexFile);
indexSegment.destroy();
// Test for scenarios by creating on-disk segment in V1 and then loading
@@ -730,7 +720,6 @@ public class LoaderTest {
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
assertTrue(textIndexFile.isDirectory());
- assertFalse(textIndexDocIdMappingFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(), new
SegmentMetadataImpl(_indexDir).getName());
@@ -748,17 +737,12 @@ public class LoaderTest {
assertFalse(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir,
SegmentVersion.v3).exists());
// no change/conversion should have happened in text index Dir
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
- // segment load should have created the docID mapping file in V1 structure
- textIndexDocIdMappingFile =
SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
- assertNotNull(textIndexDocIdMappingFile);
assertTrue(textIndexFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(), new
SegmentMetadataImpl(_indexDir).getName());
- assertEquals(textIndexDocIdMappingFile.getName(),
- TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- assertEquals(textIndexDocIdMappingFile.getParentFile().getName(), new
SegmentMetadataImpl(_indexDir).getName());
+ assertIdentityTextIndexMapping(indexSegment, textIndexFile);
indexSegment.destroy();
// CASE 2: set the segment version to load in IndexLoadingConfig to V1
@@ -774,17 +758,12 @@ public class LoaderTest {
assertFalse(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir,
SegmentVersion.v3).exists());
// no change/conversion should have happened in text index Dir
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
- // segment load should have created the docID mapping file in V1 structure
- textIndexDocIdMappingFile =
SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
- assertNotNull(textIndexDocIdMappingFile);
assertTrue(textIndexFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(), new
SegmentMetadataImpl(_indexDir).getName());
- assertEquals(textIndexDocIdMappingFile.getName(),
- TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- assertEquals(textIndexDocIdMappingFile.getParentFile().getName(), new
SegmentMetadataImpl(_indexDir).getName());
+ assertIdentityTextIndexMapping(indexSegment, textIndexFile);
indexSegment.destroy();
// CASE 3: set the segment version to load in IndexLoadingConfig to V3
@@ -800,20 +779,27 @@ public class LoaderTest {
verifyIndexDirIsV3(_indexDir);
// check that text index exists under V3 subdir. It should exist and
should be a subdir
textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir,
TEXT_INDEX_COL_NAME);
- // segment load should have created the docID mapping file in V3 structure
- textIndexDocIdMappingFile =
SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME);
assertNotNull(textIndexFile);
- assertNotNull(textIndexDocIdMappingFile);
assertTrue(textIndexFile.isDirectory());
assertEquals(textIndexFile.getName(),
TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
assertEquals(textIndexFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
- assertEquals(textIndexDocIdMappingFile.getName(),
- TEXT_INDEX_COL_NAME +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION);
- assertEquals(textIndexDocIdMappingFile.getParentFile().getName(),
SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ assertIdentityTextIndexMapping(indexSegment, textIndexFile);
indexSegment.destroy();
}
+ private void assertIdentityTextIndexMapping(IndexSegment indexSegment, File
textIndexDir)
+ throws Exception {
+ // Verified identity mappings remain Skip across loads and V1-to-V3
conversion, without a sidecar.
+ assertNull(SegmentDirectoryPaths.findTextIndexDocIdMappingFile(_indexDir,
TEXT_INDEX_COL_NAME));
+ File propertiesFile = new File(textIndexDir,
V1Constants.Indexes.LUCENE_TEXT_INDEX_PROPERTIES_FILE);
+
assertEquals(TextIndexUtils.getUpdatedConfigFromPropertiesFile(propertiesFile,
+ new TextIndexConfigBuilder().build())
+ .getDocIdTranslatorMode(), DocIdTranslatorMode.Skip);
+
assertEquals(indexSegment.getDataSource(TEXT_INDEX_COL_NAME).getTextIndex().getDocIds("*:*",
null).getCardinality(),
+ indexSegment.getSegmentMetadata().getTotalDocs());
+ }
+
private void constructSegmentWithVectorIndex(SegmentVersion segmentVersion)
throws Exception {
FileUtils.deleteQuietly(INDEX_DIR);
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexRollbackRecoveryTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexRollbackRecoveryTest.java
index 453278715b5..88438678440 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexRollbackRecoveryTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexRollbackRecoveryTest.java
@@ -356,7 +356,7 @@ public class TextIndexRollbackRecoveryTest implements
PinotBuffersAfterClassChec
new
IndexWriterConfig(analyzer).setOpenMode(IndexWriterConfig.OpenMode.CREATE))) {
Document document = new Document();
document.add(new TextField("text", "partial", Field.Store.NO));
- document.add(new
StoredField(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME, 0));
+ document.add(new
StoredField(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME, 0));
writer.addDocument(document);
}
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCompatibleTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCompatibleTest.java
index 89bdc74a460..4b41fd6439c 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCompatibleTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCompatibleTest.java
@@ -20,12 +20,50 @@ package
org.apache.pinot.segment.local.segment.index.readers.text;
import java.io.File;
import java.io.IOException;
+import java.nio.ByteOrder;
+import java.nio.file.Files;
import java.util.Map;
-import org.testng.Assert;
+import org.apache.commons.io.FileUtils;
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
+import org.apache.lucene.document.NumericDocValuesField;
+import org.apache.lucene.document.StoredField;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.document.TextField;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.NoMergePolicy;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCombined;
+import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
+import
org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
+import org.apache.pinot.segment.spi.V1Constants;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.apache.pinot.segment.spi.index.TextIndexConfig.DocIdTranslatorMode;
+import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
public class LuceneTextIndexCompatibleTest {
+ private static final String COLUMN = "Text";
+ private static final int[] PINOT_IDS = {2, 0, 3, 1};
@Test
public void testLucene80IndexReader()
@@ -33,7 +71,139 @@ public class LuceneTextIndexCompatibleTest {
File indexPath =
new
File(LuceneTextIndexCompatibleTest.class.getClassLoader().getResource("data/lucene_80_index").getPath());
try (LuceneTextIndexReader lucene80Index = new
LuceneTextIndexReader("Text", indexPath, 1000, Map.of())) {
- Assert.assertNotNull(lucene80Index);
+ assertNotNull(lucene80Index);
+ }
+ }
+
+ @Test
+ public void testDocValuesMappingDoesNotReadStoredDocuments()
+ throws IOException {
+ File segmentDir =
Files.createTempDirectory("lucene-doc-values-mapping").toFile();
+ try {
+ File indexDir = writeMappedIndex(segmentDir, "all", false, false);
+ try (Directory directory = FSDirectory.open(indexDir.toPath());
+ DirectoryReader reader = spy(DirectoryReader.open(directory));
+ PinotDataBuffer buffer =
PinotDataBuffer.allocateDirect(PINOT_IDS.length * Integer.BYTES,
+ ByteOrder.LITTLE_ENDIAN, "doc-id mapping test")) {
+ assertTrue(reader.leaves().size() > 1);
+ assertFalse(TextIndexUtils.fillDocIdMapping(reader, buffer,
PINOT_IDS.length, null));
+ for (int i = 0; i < PINOT_IDS.length; i++) {
+ assertEquals(buffer.getInt(i * Integer.BYTES), PINOT_IDS[i]);
+ }
+ // Remapping the permuted mutable IDs back to Lucene order creates an
identity mapping.
+ assertTrue(TextIndexUtils.fillDocIdMapping(reader, buffer,
PINOT_IDS.length, new int[]{1, 3, 0, 2}));
+ for (int i = 0; i < PINOT_IDS.length; i++) {
+ assertEquals(buffer.getInt(i * Integer.BYTES), i);
+ }
+ verify(reader, never()).storedFields();
+ }
+ } finally {
+ FileUtils.deleteDirectory(segmentDir);
+ }
+ }
+
+ @DataProvider(name = "mappingFormats")
+ public Object[][] mappingFormats() {
+ return new Object[][]{
+ {"none", false, DocIdTranslatorMode.Default, false},
+ {"mixed", false, DocIdTranslatorMode.Default, false},
+ {"all", false, DocIdTranslatorMode.Default, false},
+ {"none", true, DocIdTranslatorMode.TryOptimize, false},
+ {"mixed", true, DocIdTranslatorMode.TryOptimize, false},
+ {"all", true, DocIdTranslatorMode.TryOptimize, false},
+ {"all", false, DocIdTranslatorMode.TryOptimize, true}
+ };
+ }
+
+ @Test(dataProvider = "mappingFormats")
+ public void testLegacyAndMixedDocValuesMapping(String docValuesMode, boolean
deleteDocument,
+ DocIdTranslatorMode translatorMode, boolean sortIndex)
+ throws IOException {
+ File segmentDir =
Files.createTempDirectory("lucene-mixed-doc-values").toFile();
+ try {
+ writeMappedIndex(segmentDir, docValuesMode, deleteDocument, sortIndex);
+ TextIndexConfig config =
+ new
TextIndexConfigBuilder().withDocIdTranslatorMode(translatorMode.name()).build();
+ // Document ids are permuted and span multiple leaves. A deletion must
not shift the remaining mappings.
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(COLUMN,
segmentDir, PINOT_IDS.length, config)) {
+ assertEquals(reader.getDocIds("tokenzero", null).toArray(), new
int[]{2});
+ assertEquals(reader.getDocIds("tokentwo", null).toArray(), new
int[]{3});
+ assertEquals(reader.getDocIds("tokenthree", null).toArray(), new
int[]{1});
+ assertEquals(reader.getDocIds("matchall", null).toArray(),
+ deleteDocument ? new int[]{1, 2, 3} : new int[]{0, 1, 2, 3});
+ }
+ // A reload uses the persisted mapping generated from whichever
representation each leaf provided.
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(COLUMN,
segmentDir, PINOT_IDS.length, config)) {
+ assertEquals(reader.getDocIds("matchall", null).toArray(),
+ deleteDocument ? new int[]{1, 2, 3} : new int[]{0, 1, 2, 3});
+ }
+ } finally {
+ FileUtils.deleteDirectory(segmentDir);
+ }
+ }
+
+ @Test
+ public void testLegacyCombinedIndexWithoutMappingBuffer()
+ throws IOException {
+ File segmentDir =
Files.createTempDirectory("lucene-legacy-combined").toFile();
+ try {
+ File indexDir = writeMappedIndex(segmentDir, "none", false, false);
+ File combinedFile = new File(segmentDir, COLUMN +
V1Constants.Indexes.LUCENE_COMBINE_TEXT_INDEX_FILE_EXTENSION);
+ LuceneTextIndexCombined.combineLuceneIndexFiles(indexDir,
combinedFile.toString());
+ FileUtils.deleteDirectory(indexDir);
+ TextIndexConfig config = new
TextIndexConfigBuilder().setStoreInSegmentFile(true)
+ .withDocIdTranslatorMode(DocIdTranslatorMode.Default.name()).build();
+ try (PinotDataBuffer buffer = PinotDataBuffer.mapFile(combinedFile,
true, 0, combinedFile.length(),
+ ByteOrder.LITTLE_ENDIAN, "legacy combined text index test")) {
+
assertNull(LuceneTextIndexBufferReader.extractDocIdMappingBuffer(buffer,
COLUMN));
+ try (LuceneTextIndexReader reader = new LuceneTextIndexReader(COLUMN,
buffer, PINOT_IDS.length, config)) {
+ assertEquals(reader.getDocIds("tokenzero", null).toArray(), new
int[]{2});
+ assertEquals(reader.getDocIds("tokentwo", null).toArray(), new
int[]{3});
+ assertEquals(reader.getDocIds("matchall", null).toArray(), new
int[]{0, 1, 2, 3});
+ }
+ }
+ } finally {
+ FileUtils.deleteDirectory(segmentDir);
+ }
+ }
+
+ private static File writeMappedIndex(File segmentDir, String docValuesMode,
boolean deleteDocument, boolean sortIndex)
+ throws IOException {
+ File indexDir = new File(segmentDir, COLUMN +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION);
+ String[] tokens = {"tokenzero", "tokenone", "tokentwo", "tokenthree"};
+ IndexWriterConfig writerConfig = new IndexWriterConfig(new
StandardAnalyzer());
+ if (sortIndex) {
+ writerConfig.setIndexSort(new Sort(new SortField("order",
SortField.Type.INT, true)));
+ } else {
+ writerConfig.setMergePolicy(NoMergePolicy.INSTANCE);
+ }
+ try (Directory directory = FSDirectory.open(indexDir.toPath());
+ IndexWriter writer = new IndexWriter(directory, writerConfig)) {
+ for (int i = 0; i < PINOT_IDS.length; i++) {
+ Document document = new Document();
+ document.add(new TextField(COLUMN, "matchall " + tokens[i],
Field.Store.NO));
+ document.add(new StringField("key", tokens[i], Field.Store.NO));
+ document.add(new
StoredField(LuceneTextIndexCreator.PINOT_DOC_ID_STORED_FIELD_NAME,
PINOT_IDS[i]));
+ if (docValuesMode.equals("all") || docValuesMode.equals("mixed") && i
== 1) {
+ document.add(
+ new
NumericDocValuesField(LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME,
PINOT_IDS[i]));
+ }
+ if (sortIndex) {
+ document.add(new NumericDocValuesField("order", i));
+ }
+ writer.addDocument(document);
+ if (i % 2 == 1) {
+ writer.commit();
+ }
+ }
+ if (deleteDocument) {
+ writer.deleteDocuments(new Term("key", tokens[1]));
+ writer.commit();
+ }
+ if (sortIndex) {
+ writer.forceMerge(1);
+ }
}
+ return indexDir;
}
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
index 92ba615cb22..5f1a8c9489e 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexCountTest.java
@@ -55,6 +55,7 @@ import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
/// Covers the `count(*)` short-circuit:
[LuceneTextIndexReader#getNumMatchingDocs] and the
@@ -216,23 +217,20 @@ public class LuceneTextIndexCountTest {
}
}
- /// A Lucene index larger than the segment means a broken doc-id mapping.
Counting does not consult the
- /// translator, so it must refuse the fast path and fall back to the doc-id
path, which fails loudly rather
- /// than returning an inflated count.
+ /// Both query paths reject an oversized index even when a persisted Skip
setting removes translator bounds checks.
@Test
- public void testOversizedIndexFallsBackInsteadOfCountingWrong()
+ public void testOversizedIndexRejectsInvalidDocIds()
throws Exception {
int understatedNumDocs = DOCS.length - 4;
try (LuceneTextIndexReader reader =
new LuceneTextIndexReader(COLUMN, INDEX_DIR, understatedNumDocs,
Map.of())) {
- try {
- int count = reader.getNumMatchingDocs("sentinelall", null);
- assertTrue(count <= understatedNumDocs,
- "returned a count larger than the segment (" + count + " > " +
understatedNumDocs + ")");
- } catch (RuntimeException e) {
- // Falling back and failing loudly is the intended behaviour for a
broken mapping. The fallback runs
- // getDocIds, whose translator throws, and getNumMatchingDocs rewraps
it with its own wording.
- assertTrue(e.getMessage().contains("text index"), "unexpected failure:
" + e.getMessage());
+ RuntimeException countFailure =
+ expectThrows(RuntimeException.class, () ->
reader.getNumMatchingDocs("sentinelall", null));
+ RuntimeException idsFailure = expectThrows(RuntimeException.class, () ->
reader.getDocIds("sentinelall", null));
+ for (RuntimeException failure : new RuntimeException[]{countFailure,
idsFailure}) {
+ assertTrue(failure.getMessage().contains("text index"));
+ assertTrue(failure.getCause() instanceof IllegalStateException);
+ assertTrue(failure.getCause().getMessage().contains("exceeding segment
document count"));
}
}
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/FilePerIndexDirectoryTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/FilePerIndexDirectoryTest.java
index 8254afa938d..a2364555d9b 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/FilePerIndexDirectoryTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/FilePerIndexDirectoryTest.java
@@ -28,6 +28,7 @@ import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import org.apache.commons.io.FileUtils;
+import org.apache.lucene.index.Term;
import org.apache.pinot.segment.local.PinotBuffersAfterMethodCheckRule;
import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
import
org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexReader;
@@ -204,24 +205,36 @@ public class FilePerIndexDirectoryTest implements
PinotBuffersAfterMethodCheckRu
buf.putChar(111, 'h');
fooCreator.add("{\"clean\":\"this\"}");
+ fooCreator.add("{\"other\":\"that\"}");
+ // Keep valid nonidentity mappings so removal covers both the index and
its required sidecar.
+ fooCreator.getIndexWriter().updateNumericDocValue(new Term("foo",
"clean"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 1);
+ fooCreator.getIndexWriter().updateNumericDocValue(new Term("foo",
"other"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 0);
fooCreator.seal();
barCreator.add("{\"retain\":\"this\"}");
barCreator.add("{\"keep\":\"this\"}");
barCreator.add("{\"hold\":\"this\"}");
+ barCreator.getIndexWriter().updateNumericDocValue(new Term("bar",
"retain"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 2);
+ barCreator.getIndexWriter().updateNumericDocValue(new Term("bar",
"hold"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 0);
barCreator.seal();
}
// Remove the Text index to trigger cleanup.
try (FilePerIndexDirectory fpi = new FilePerIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
assertTrue(fpi.hasIndexFor("foo", StandardIndexes.text()));
- // Use TextIndex once to trigger the creation of mapping files.
- try (LuceneTextIndexReader fooReader = new LuceneTextIndexReader("foo",
TEMP_DIR, 1, new HashMap<>())) {
- fooReader.getDocIds("clean");
+ // Verify that the persisted mappings translate the permuted IDs before
cleanup.
+ try (LuceneTextIndexReader fooReader = new LuceneTextIndexReader("foo",
TEMP_DIR, 2, new HashMap<>())) {
+ assertEquals(fooReader.getDocIds("clean", null).toArray(), new
int[]{1});
}
try (LuceneTextIndexReader barReader = new LuceneTextIndexReader("bar",
TEMP_DIR, 3, new HashMap<>())) {
- barReader.getDocIds("retain hold");
+ assertEquals(barReader.getDocIds("retain", null).toArray(), new
int[]{2});
}
+ assertTrue(
+ new File(TEMP_DIR, "foo" +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION).exists());
// Both files for TextIndex should be removed.
fpi.removeIndex("foo", StandardIndexes.text());
assertFalse(new File(TEMP_DIR, "foo" +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION).exists());
@@ -248,6 +261,7 @@ public class FilePerIndexDirectoryTest implements
PinotBuffersAfterMethodCheckRu
// Check if the text index still work.
try (LuceneTextIndexReader barReader = new LuceneTextIndexReader("bar",
TEMP_DIR, 3, new HashMap<>())) {
+ assertEquals(barReader.getDocIds("retain", null).toArray(), new
int[]{2});
MutableRoaringBitmap ids = barReader.getDocIds("retain hold");
assertTrue(ids.contains(0));
assertTrue(ids.contains(2));
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
index 3825363d5f9..4b620248297 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
@@ -34,6 +34,7 @@ import java.util.UUID;
import java.util.stream.Collectors;
import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.commons.io.FileUtils;
+import org.apache.lucene.index.Term;
import org.apache.pinot.segment.local.PinotBuffersAfterMethodCheckRule;
import
org.apache.pinot.segment.local.segment.creator.impl.text.LuceneTextIndexCreator;
import
org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexReader;
@@ -252,24 +253,36 @@ public class SingleFileIndexDirectoryTest implements
PinotBuffersAfterMethodChec
buf.putChar(111, 'h');
fooCreator.add("{\"clean\":\"this\"}");
+ fooCreator.add("{\"other\":\"that\"}");
+ // Keep valid nonidentity mappings so removal covers both the index and
its required sidecar.
+ fooCreator.getIndexWriter().updateNumericDocValue(new Term("foo",
"clean"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 1);
+ fooCreator.getIndexWriter().updateNumericDocValue(new Term("foo",
"other"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 0);
fooCreator.seal();
barCreator.add("{\"retain\":\"this\"}");
barCreator.add("{\"keep\":\"this\"}");
barCreator.add("{\"hold\":\"this\"}");
+ barCreator.getIndexWriter().updateNumericDocValue(new Term("bar",
"retain"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 2);
+ barCreator.getIndexWriter().updateNumericDocValue(new Term("bar",
"hold"),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME, 0);
barCreator.seal();
}
// Remove the Text index to trigger cleanup.
try (SingleFileIndexDirectory sfd = new SingleFileIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
assertTrue(sfd.hasIndexFor("foo", StandardIndexes.text()));
- // Use TextIndex once to trigger the creation of mapping files.
- try (LuceneTextIndexReader fooReader = new LuceneTextIndexReader("foo",
TEMP_DIR, 1, new HashMap<>())) {
- fooReader.getDocIds("clean");
+ // Verify that the persisted mappings translate the permuted IDs before
cleanup.
+ try (LuceneTextIndexReader fooReader = new LuceneTextIndexReader("foo",
TEMP_DIR, 2, new HashMap<>())) {
+ assertEquals(fooReader.getDocIds("clean", null).toArray(), new
int[]{1});
}
try (LuceneTextIndexReader barReader = new LuceneTextIndexReader("bar",
TEMP_DIR, 3, new HashMap<>())) {
- barReader.getDocIds("retain hold");
+ assertEquals(barReader.getDocIds("retain", null).toArray(), new
int[]{2});
}
+ assertTrue(
+ new File(TEMP_DIR, "foo" +
V1Constants.Indexes.LUCENE_TEXT_INDEX_DOCID_MAPPING_FILE_EXTENSION).exists());
// Both files for TextIndex should be removed.
sfd.removeIndex("foo", StandardIndexes.text());
assertFalse(new File(TEMP_DIR, "foo" +
V1Constants.Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION).exists());
@@ -296,6 +309,7 @@ public class SingleFileIndexDirectoryTest implements
PinotBuffersAfterMethodChec
// Check if the text index still work.
try (LuceneTextIndexReader barReader = new LuceneTextIndexReader("bar",
TEMP_DIR, 3, new HashMap<>())) {
+ assertEquals(barReader.getDocIds("retain", null).toArray(), new
int[]{2});
MutableRoaringBitmap ids = barReader.getDocIds("retain hold");
assertTrue(ids.contains(0));
assertTrue(ids.contains(2));
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/TextIndexConfig.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/TextIndexConfig.java
index 347aaaa146a..47e6b806009 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/TextIndexConfig.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/TextIndexConfig.java
@@ -78,11 +78,11 @@ public class TextIndexConfig extends IndexConfig {
private final boolean _storeInSegmentFile;
public enum DocIdTranslatorMode {
- // build and keep mapping
+ /// Keep load-time and reused-mutable mappings. Fresh immutable indexes
still drop verified identity mappings.
Default,
- // build mapping and check if it can be dropped
+ /// Drop verified identity mappings, including when reusing mutable
indexes.
TryOptimize,
- // mapping is not necessary, use lucene ids
+ /// Use Lucene document IDs without constructing or verifying a mapping.
Skip;
static DocIdTranslatorMode of(String mode) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
