xiangfu0 commented on code in PR #19897:
URL: https://github.com/apache/pinot/pull/19897#discussion_r4372995873
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java:
##########
@@ -237,55 +241,47 @@ private void convertMutableSegment(File segmentIndexDir,
File consumerDir, @Null
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 older readers.
/// @param segmentIndexDir segment index directory
/// @param column column name
/// @param directory directory of the index
/// @param mutableToImmutableDocIdMap mutableToImmutableDocIdMap from
segment conversion
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);
+ }
Review Comment:
Clarified in
[22abe15f57](https://github.com/apache/pinot/commit/22abe15f57d9dcf8b1f360484f464e22a2dd239e):
`Default` keeps load-time/reused-mutable mappings, while fresh immutable
indexes persist `Skip` for verified identity mappings. The enum now documents
this existing behavior; no executable code changed. The existing
explicit-Default identity test already covers it.
The description and writer documentation now state the Pinot 1.4.0 minimum
reader version, including server-first upgrade and rollback constraints. The
1.3.0/1.4.0 reader sources were checked; the executed compatibility evidence
remains the precisely described 1.5.0-rc0 shim probe. All four affected-module
formatting/license/style checks and independent review passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
