You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by si...@apache.org on 2020/02/21 08:35:43 UTC
[incubator-pinot] branch master updated: Support text index load
for V1 segment format (#5085)
This is an automated email from the ASF dual-hosted git repository.
siddteotia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 21850ba Support text index load for V1 segment format (#5085)
21850ba is described below
commit 21850ba17132c52f175d85930ca561849a1218b5
Author: Sidd <si...@gmail.com>
AuthorDate: Fri Feb 21 00:35:33 2020 -0800
Support text index load for V1 segment format (#5085)
* Support text index load for V1 segment format
* Fix formatting
---
.../impl/inv/text/LuceneTextIndexCreator.java | 11 +-
.../index/readers/text/LuceneTextIndexReader.java | 27 +++-
.../core/segment/store/SegmentDirectoryPaths.java | 13 ++
.../core/segment/index/loader/LoaderTest.java | 155 +++++++++++++++++++++
4 files changed, 200 insertions(+), 6 deletions(-)
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/text/LuceneTextIndexCreator.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/text/LuceneTextIndexCreator.java
index ba93240..2d7ecf9 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/text/LuceneTextIndexCreator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/text/LuceneTextIndexCreator.java
@@ -20,7 +20,6 @@ package org.apache.pinot.core.segment.creator.impl.inv.text;
import java.io.File;
import java.io.IOException;
-import java.util.logging.Logger;
import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
@@ -31,7 +30,6 @@ import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
import org.apache.pinot.core.segment.creator.InvertedIndexCreator;
-import org.apache.pinot.core.segment.creator.impl.SegmentColumnarIndexCreator;
import org.slf4j.LoggerFactory;
@@ -79,7 +77,9 @@ public class LuceneTextIndexCreator implements InvertedIndexCreator {
public LuceneTextIndexCreator(String column, File segmentIndexDir, boolean commit) {
_textColumn = column;
try {
- File indexFile = new File(segmentIndexDir.getPath() + "/" + _textColumn + LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ // segment generation is always in V1 and later we convert (as part of post creation processing)
+ // to V3 if segmentVersion is set to V3 in SegmentGeneratorConfig.
+ File indexFile = getV1TextIndexFile(segmentIndexDir);
_indexDirectory = FSDirectory.open(indexFile.toPath());
StandardAnalyzer standardAnalyzer = new StandardAnalyzer();
IndexWriterConfig indexWriterConfig = new IndexWriterConfig(standardAnalyzer);
@@ -155,4 +155,9 @@ public class LuceneTextIndexCreator implements InvertedIndexCreator {
public void add(int[] dictIds, int length) {
throw new IllegalStateException("Lucene text inverted index is not dictionary based");
}
+
+ private File getV1TextIndexFile(File indexDir) {
+ String luceneIndexDirectory = _textColumn + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION;
+ return new File(indexDir, luceneIndexDirectory);
+ }
}
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
index 12e1446..abefe15 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
@@ -30,9 +30,9 @@ 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.core.indexsegment.generator.SegmentVersion;
import org.apache.pinot.core.segment.creator.impl.inv.text.LuceneTextIndexCreator;
import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.store.SegmentDirectoryPaths;
import org.roaringbitmap.IntIterator;
import org.roaringbitmap.buffer.MutableRoaringBitmap;
import org.slf4j.LoggerFactory;
@@ -63,8 +63,7 @@ public class LuceneTextIndexReader implements InvertedIndexReader<MutableRoaring
public LuceneTextIndexReader(String column, File segmentIndexDir) {
_column = column;
try {
- File indexFile = new File(segmentIndexDir.getPath() + "/" + SegmentVersion.v3.name() + "/" + column
- + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ File indexFile = getTextIndexFile(segmentIndexDir);
_indexDirectory = FSDirectory.open(indexFile.toPath());
_indexReader = DirectoryReader.open(_indexDirectory);
_indexSearcher = new IndexSearcher(_indexReader);
@@ -80,6 +79,28 @@ public class LuceneTextIndexReader implements InvertedIndexReader<MutableRoaring
_queryParser = new QueryParser(column, analyzer);
}
+ /**
+ * CASE 1: If IndexLoadingConfig specifies a segment version to load and if it is different then
+ * the on-disk version of the segment, then {@link org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentLoader}
+ * will take care of up-converting the on-disk segment to v3 before load. The converter
+ * already has support for converting v1 text index to v3. So the text index can be
+ * loaded from segmentIndexDir/v3/ since v3 sub-directory would have already been created
+ *
+ * CASE 2: However, if IndexLoadingConfig doesn't specify the segment version to load or if the specified
+ * version is same as the on-disk version of the segment, then ImmutableSegmentLoader will load
+ * whatever the version of segment is on disk.
+ * @param segmentIndexDir top-level segment index directory
+ * @return text index file
+ */
+ private File getTextIndexFile(File segmentIndexDir) {
+ // will return null if file does not exist
+ File file = SegmentDirectoryPaths.findTextIndexIndexFile(segmentIndexDir, _column);
+ if (file == null) {
+ throw new IllegalStateException("Failed to find text index file for column: " + _column);
+ }
+ return file;
+ }
+
@Override
public MutableRoaringBitmap getDocIds(int dictId) {
// This should not be called from anywhere. If it happens, there is a bug in the current implementation
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/store/SegmentDirectoryPaths.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/store/SegmentDirectoryPaths.java
index cdfc1b3..75af885 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/store/SegmentDirectoryPaths.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/store/SegmentDirectoryPaths.java
@@ -24,6 +24,7 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.pinot.core.indexsegment.generator.SegmentVersion;
import org.apache.pinot.core.segment.creator.impl.V1Constants;
+import org.apache.pinot.core.segment.creator.impl.inv.text.LuceneTextIndexCreator;
public class SegmentDirectoryPaths {
@@ -79,6 +80,18 @@ public class SegmentDirectoryPaths {
}
/**
+ * Find text index file in top-level segment index directory
+ * @param indexDir top-level segment index directory
+ * @param column text column name
+ * @return text index directory (if exists in V3, V1 or V2 format), null if index file does not exit
+ */
+ @Nullable
+ public static File findTextIndexIndexFile(File indexDir, String column) {
+ String luceneIndexDirectory = column + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION;
+ return findFormatFile(indexDir, luceneIndexDirectory);
+ }
+
+ /**
* Find a file in any segment version.
* <p>Index directory passed in should be top level segment directory.
* <p>If file exists in multiple segment version, return the one in highest segment version.
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/LoaderTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/LoaderTest.java
index 6e831d3..59342a3 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/LoaderTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/LoaderTest.java
@@ -19,8 +19,13 @@
package org.apache.pinot.core.segment.index.loader;
import java.io.File;
+import java.io.FilenameFilter;
import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.FileFileFilter;
+import org.apache.pinot.core.segment.creator.impl.inv.text.LuceneTextIndexCreator;
import org.apache.pinot.spi.data.DimensionFieldSpec;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.MetricFieldSpec;
@@ -49,6 +54,7 @@ import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import org.testng.collections.Lists;
public class LoaderTest {
@@ -58,6 +64,8 @@ public class LoaderTest {
private static final String PADDING_PERCENT = "data/paddingPercent.tar.gz";
private static final String PADDING_NULL = "data/paddingNull.tar.gz";
+ private static final String TEXT_INDEX_COL_NAME = "column5";
+
private File _avroFile;
private File _indexDir;
private IndexLoadingConfig _v1IndexLoadingConfig;
@@ -244,6 +252,153 @@ public class LoaderTest {
indexSegment.destroy();
}
+ private void constructSegmentWithTextIndex(SegmentVersion segmentVersion)
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+ SegmentGeneratorConfig segmentGeneratorConfig =
+ SegmentTestUtils.getSegmentGeneratorConfigWithoutTimeColumn(_avroFile, INDEX_DIR, "testTable");
+ SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
+ List<String> textIndexCreationColumns = Lists.newArrayList(TEXT_INDEX_COL_NAME);
+ List<String> rawIndexCreationColumns = Lists.newArrayList(TEXT_INDEX_COL_NAME);
+ segmentGeneratorConfig.setTextIndexCreationColumns(textIndexCreationColumns);
+ segmentGeneratorConfig.setRawIndexCreationColumns(rawIndexCreationColumns);
+ segmentGeneratorConfig.setSegmentVersion(segmentVersion);
+ driver.init(segmentGeneratorConfig);
+ driver.build();
+
+ _indexDir = new File(INDEX_DIR, driver.getSegmentName());
+ }
+
+ @Test
+ public void testTextIndexLoad() throws Exception {
+ // create on-disk segment in V3
+ // this generates the segment in V1 but converts to V3 as part of post-creation processing
+ constructSegmentWithTextIndex(SegmentVersion.v3);
+
+ // check that segment on-disk version is V3 after creation
+ Assert.assertEquals(new SegmentMetadataImpl(_indexDir).getSegmentVersion(), SegmentVersion.v3);
+ // check that V3 index sub-dir exists
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v3).exists());
+ // check that index dir is not in V1 format (the only subdir it should have is V3)
+ verifyIndexDirIsV3(_indexDir);
+ // check that text index exists under V3 subdir.
+ File textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir, TEXT_INDEX_COL_NAME);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+
+ // CASE 1: don't set the segment version to load in IndexLoadingConfig
+ // there should be no conversion done by ImmutableSegmentLoader and it should
+ // be able to create text index reader with on-disk version V3
+ IndexSegment indexSegment = ImmutableSegmentLoader.load(_indexDir, ReadMode.mmap);
+ // check that loaded segment version is v3
+ Assert.assertEquals(indexSegment.getSegmentMetadata().getVersion(), SegmentVersion.v3.toString());
+ // no change/conversion should have happened in indexDir
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v3).exists());
+ // check that index dir is not in V1 format (the only subdir it should have is V3)
+ verifyIndexDirIsV3(_indexDir);
+ // no change/conversion should have happened for textIndex dir
+ textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir, TEXT_INDEX_COL_NAME);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ indexSegment.destroy();
+
+ // CASE 2: set the segment version to load in IndexLoadingConfig as V3
+ // there should be no conversion done by ImmutableSegmentLoader since the segmentVersionToLoad
+ // is same as the version of segment on disk (V3)
+ indexSegment = ImmutableSegmentLoader.load(_indexDir, _v3IndexLoadingConfig);
+ // check that loaded segment version is v3
+ Assert.assertEquals(indexSegment.getSegmentMetadata().getVersion(), SegmentVersion.v3.toString());
+ // no change/conversion should have happened in indexDir
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v3).exists());
+ // check that index dir is not in V1 format (the only subdir it should have is V3)
+ verifyIndexDirIsV3(_indexDir);
+ // no change/conversion should have happened for textIndex dir
+ textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir, TEXT_INDEX_COL_NAME);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ indexSegment.destroy();
+
+ // create on-disk segment in V1
+ // this generates the segment in V1 and does not convert to V3 as part of post-creation processing
+ constructSegmentWithTextIndex(SegmentVersion.v1);
+
+ // check that segment on-disk version is V1 after creation
+ Assert.assertEquals(new SegmentMetadataImpl(_indexDir).getSegmentVersion(), SegmentVersion.v1);
+ // check that segment v1 dir exists
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v1).exists());
+ // check that v3 index sub-dir does not exist
+ Assert.assertFalse(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v3).exists());
+ // check that text index exists directly under indexDir (V1). it should exist and should be a subdir
+ textIndexFile = SegmentDirectoryPaths.findTextIndexIndexFile(_indexDir, TEXT_INDEX_COL_NAME);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), new SegmentMetadataImpl(_indexDir).getName());
+
+ // CASE 1: don't set the segment version to load in IndexLoadingConfig
+ // there should be no conversion done by ImmutableSegmentLoader and it should
+ // be able to create text index reader with on-disk version V1
+ indexSegment = ImmutableSegmentLoader.load(_indexDir, ReadMode.mmap);
+ // check that loaded segment version is v1
+ Assert.assertEquals(indexSegment.getSegmentMetadata().getVersion(), SegmentVersion.v1.toString());
+ // no change/conversion should have happened in indexDir
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v1).exists());
+ Assert.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);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), new SegmentMetadataImpl(_indexDir).getName());
+ indexSegment.destroy();
+
+ // CASE 2: set the segment version to load in IndexLoadingConfig to V1
+ // there should be no conversion done by ImmutableSegmentLoader since the segmentVersionToLoad
+ // is same as the version of segment on fisk
+ indexSegment = ImmutableSegmentLoader.load(_indexDir, _v1IndexLoadingConfig);
+ // check that loaded segment version is v1
+ Assert.assertEquals(indexSegment.getSegmentMetadata().getVersion(), SegmentVersion.v1.toString());
+ // no change/conversion should have happened in indexDir
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v1).exists());
+ Assert.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);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), new SegmentMetadataImpl(_indexDir).getName());
+ indexSegment.destroy();
+
+ // CASE 3: set the segment version to load in IndexLoadingConfig to V3
+ // there should be conversion done by ImmutableSegmentLoader since the segmentVersionToLoad
+ // is different than the version of segment on disk
+ indexSegment = ImmutableSegmentLoader.load(_indexDir, _v3IndexLoadingConfig);
+ // check that loaded segment version is v3
+ Assert.assertEquals(indexSegment.getSegmentMetadata().getVersion(), SegmentVersion.v3.toString());
+ // the index dir should exist in v3 format due to conversion
+ Assert.assertTrue(SegmentDirectoryPaths.segmentDirectoryFor(_indexDir, SegmentVersion.v3).exists());
+ 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);
+ Assert.assertNotNull(textIndexFile);
+ Assert.assertTrue(textIndexFile.isDirectory());
+ Assert.assertEquals(textIndexFile.getName(), TEXT_INDEX_COL_NAME + LuceneTextIndexCreator.LUCENE_TEXT_INDEX_FILE_EXTENSION);
+ Assert.assertEquals(textIndexFile.getParentFile().getName(), SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ indexSegment.destroy();
+ }
+
+ private void verifyIndexDirIsV3(File indexDir) {
+ File[] files = indexDir.listFiles();
+ Assert.assertEquals(files.length, 1);
+ Assert.assertEquals(files[0].getName(), SegmentDirectoryPaths.V3_SUBDIRECTORY_NAME);
+ }
+
@AfterClass
public void tearDown() {
FileUtils.deleteQuietly(INDEX_DIR);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org