This is an automated email from the ASF dual-hosted git repository.
jackie 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 94b9ca3 cleanup segment preprocessor a bit to make next RP smaller
(#7294)
94b9ca3 is described below
commit 94b9ca326b398105fcf1a19381070a86f014d51b
Author: Xiaobing <61892277+klsi...@users.noreply.github.com>
AuthorDate: Thu Aug 19 15:03:44 2021 -0700
cleanup segment preprocessor a bit to make next RP smaller (#7294)
Break down large method to smaller ones to be more readable and reduce the
scope of variables with similar names.
---
.../immutable/ImmutableSegmentLoader.java | 36 +++----
.../segment/index/loader/SegmentPreProcessor.java | 108 ++++++++++++---------
2 files changed, 80 insertions(+), 64 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
index 7ca7246..9557a2f 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
@@ -39,7 +39,6 @@ import
org.apache.pinot.segment.spi.converter.SegmentFormatConverter;
import org.apache.pinot.segment.spi.creator.SegmentVersion;
import org.apache.pinot.segment.spi.index.column.ColumnIndexContainer;
import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl;
-import org.apache.pinot.segment.spi.loader.SegmentDirectoryLoader;
import org.apache.pinot.segment.spi.loader.SegmentDirectoryLoaderRegistry;
import org.apache.pinot.segment.spi.store.SegmentDirectory;
import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
@@ -104,24 +103,16 @@ public class ImmutableSegmentLoader {
return new EmptyIndexSegment(localSegmentMetadata);
}
- PinotConfiguration tierConfigs = indexLoadingConfig.getTierConfigs();
- PinotConfiguration segmentDirectoryLoaderConfigs = new
PinotConfiguration(tierConfigs.toMap());
-
- // Pre-process the segment on local using local SegmentDirectory
- SegmentDirectory localSegmentDirectory =
SegmentDirectoryLoaderRegistry.getLocalSegmentDirectoryLoader()
- .load(indexDir.toURI(), segmentDirectoryLoaderConfigs);
-
- // NOTE: this step may modify the segment metadata
- try (
- SegmentPreProcessor preProcessor = new
SegmentPreProcessor(localSegmentDirectory, indexLoadingConfig, schema)) {
- preProcessor.process();
- }
+ // Preprocess the segment on local using local SegmentDirectory.
+ // Please note that this step may modify the segment metadata.
+ preprocessSegment(indexDir, indexLoadingConfig, schema);
// Load the segment again for the configured tier backend. Default is
'local'.
- SegmentDirectoryLoader segmentLoaderDirectory =
-
SegmentDirectoryLoaderRegistry.getSegmentDirectoryLoader(indexLoadingConfig.getTierBackend());
+ PinotConfiguration tierConfigs = indexLoadingConfig.getTierConfigs();
+ PinotConfiguration segDirConfigs = new
PinotConfiguration(tierConfigs.toMap());
SegmentDirectory actualSegmentDirectory =
- segmentLoaderDirectory.load(indexDir.toURI(),
segmentDirectoryLoaderConfigs);
+
SegmentDirectoryLoaderRegistry.getSegmentDirectoryLoader(indexLoadingConfig.getTierBackend())
+ .load(indexDir.toURI(), segDirConfigs);
SegmentDirectory.Reader segmentReader =
actualSegmentDirectory.createReader();
SegmentMetadataImpl segmentMetadata =
actualSegmentDirectory.getSegmentMetadata();
@@ -169,7 +160,18 @@ public class ImmutableSegmentLoader {
ImmutableSegmentImpl segment =
new ImmutableSegmentImpl(actualSegmentDirectory, segmentMetadata,
indexContainerMap, starTreeIndexContainer);
- LOGGER.info("Successfully loaded segment {} with config: {}", segmentName,
segmentDirectoryLoaderConfigs);
+ LOGGER.info("Successfully loaded segment {} with config: {}", segmentName,
segDirConfigs);
return segment;
}
+
+ private static void preprocessSegment(File indexDir, IndexLoadingConfig
indexLoadingConfig, Schema schema)
+ throws Exception {
+ PinotConfiguration tierConfigs = indexLoadingConfig.getTierConfigs();
+ PinotConfiguration segDirConfigs = new
PinotConfiguration(tierConfigs.toMap());
+ SegmentDirectory segDir =
+
SegmentDirectoryLoaderRegistry.getLocalSegmentDirectoryLoader().load(indexDir.toURI(),
segDirConfigs);
+ try (SegmentPreProcessor preProcessor = new SegmentPreProcessor(segDir,
indexLoadingConfig, schema)) {
+ preProcessor.process();
+ }
+ }
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
index 8b1347f..6ed1da6 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
@@ -64,8 +64,8 @@ public class SegmentPreProcessor implements AutoCloseable {
private final SegmentDirectory _segmentDirectory;
private SegmentMetadataImpl _segmentMetadata;
- public SegmentPreProcessor(SegmentDirectory segmentDirectory,
IndexLoadingConfig indexLoadingConfig, @Nullable Schema schema)
- throws Exception {
+ public SegmentPreProcessor(SegmentDirectory segmentDirectory,
IndexLoadingConfig indexLoadingConfig,
+ @Nullable Schema schema) {
_segmentDirectory = segmentDirectory;
_indexDir = new File(segmentDirectory.getIndexDir());
_indexLoadingConfig = indexLoadingConfig;
@@ -73,24 +73,21 @@ public class SegmentPreProcessor implements AutoCloseable {
_segmentMetadata = segmentDirectory.getSegmentMetadata();
}
+ @Override
+ public void close()
+ throws Exception {
+ _segmentDirectory.close();
+ }
+
public void process()
throws Exception {
if (_segmentMetadata.getTotalDocs() == 0) {
LOGGER.info("Skip preprocessing empty segment: {}",
_segmentMetadata.getName());
return;
}
- // Remove all the existing inverted index temp files before loading
segments.
- // NOTE: This step fixes the issue of temporary files not getting deleted
after creating new inverted indexes.
- // In this, we look for all files in the directory and remove the ones
with '.bitmap.inv.tmp' extension.
- File[] directoryListing = _indexDir.listFiles();
- String tempFileExtension =
V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION + ".tmp";
- if (directoryListing != null) {
- for (File child : directoryListing) {
- if (child.getName().endsWith(tempFileExtension)) {
- FileUtils.deleteQuietly(child);
- }
- }
- }
+
+ // This fixes the issue of temporary files not getting deleted after
creating new inverted indexes.
+ removeInvertedIndexTempFiles();
try (SegmentDirectory.Writer segmentWriter =
_segmentDirectory.createWriter()) {
// Update default columns according to the schema.
@@ -146,35 +143,8 @@ public class SegmentPreProcessor implements AutoCloseable {
new BloomFilterHandler(_indexDir, _segmentMetadata,
_indexLoadingConfig, segmentWriter);
bloomFilterHandler.createBloomFilters();
- // Create/modify/remove star-trees if required
- if (_indexLoadingConfig.isEnableDynamicStarTreeCreation()) {
- List<StarTreeV2BuilderConfig> starTreeBuilderConfigs =
StarTreeBuilderUtils
-
.generateBuilderConfigs(_indexLoadingConfig.getStarTreeIndexConfigs(),
- _indexLoadingConfig.isEnableDefaultStarTree(),
_segmentMetadata);
- boolean shouldGenerateStarTree = !starTreeBuilderConfigs.isEmpty();
- List<StarTreeV2Metadata> starTreeMetadataList =
_segmentMetadata.getStarTreeV2MetadataList();
- if (starTreeMetadataList != null) {
- // There are existing star-trees
- if
(StarTreeBuilderUtils.shouldRemoveExistingStarTrees(starTreeBuilderConfigs,
starTreeMetadataList)) {
- // Remove the existing star-trees
- LOGGER.info("Removing star-trees from segment: {}",
_segmentMetadata.getName());
- StarTreeBuilderUtils.removeStarTrees(_indexDir);
- _segmentMetadata = new SegmentMetadataImpl(_indexDir);
- } else {
- // Existing star-trees match the builder configs, no need to
generate the star-trees
- shouldGenerateStarTree = false;
- }
- }
- // Generate the star-trees if needed
- if (shouldGenerateStarTree) {
- // NOTE: Always use OFF_HEAP mode on server side.
- try (MultipleTreesBuilder builder = new
MultipleTreesBuilder(starTreeBuilderConfigs, _indexDir,
- MultipleTreesBuilder.BuildMode.OFF_HEAP)) {
- builder.build();
- }
- _segmentMetadata = new SegmentMetadataImpl(_indexDir);
- }
- }
+ // Create/modify/remove star-trees if required.
+ processStarTrees();
// Add min/max value to column metadata according to the prune mode.
// For star-tree index, because it can only increase the range, so
min/max value can still be used in pruner.
@@ -185,16 +155,60 @@ public class SegmentPreProcessor implements AutoCloseable
{
new ColumnMinMaxValueGenerator(_segmentMetadata, segmentWriter,
columnMinMaxValueGeneratorMode);
columnMinMaxValueGenerator.addColumnMinMaxValue();
// NOTE: This step may modify the segment metadata. When adding new
steps after this, un-comment the next line.
-// _segmentMetadata = new SegmentMetadataImpl(_indexDir);
+ // _segmentMetadata = new SegmentMetadataImpl(_indexDir);
}
segmentWriter.save();
}
}
- @Override
- public void close()
+ private void processStarTrees()
throws Exception {
- _segmentDirectory.close();
+ // Create/modify/remove star-trees if required
+ if (_indexLoadingConfig.isEnableDynamicStarTreeCreation()) {
+ List<StarTreeV2BuilderConfig> starTreeBuilderConfigs =
StarTreeBuilderUtils
+
.generateBuilderConfigs(_indexLoadingConfig.getStarTreeIndexConfigs(),
+ _indexLoadingConfig.isEnableDefaultStarTree(), _segmentMetadata);
+ boolean shouldGenerateStarTree = !starTreeBuilderConfigs.isEmpty();
+ List<StarTreeV2Metadata> starTreeMetadataList =
_segmentMetadata.getStarTreeV2MetadataList();
+ if (starTreeMetadataList != null) {
+ // There are existing star-trees
+ if
(StarTreeBuilderUtils.shouldRemoveExistingStarTrees(starTreeBuilderConfigs,
starTreeMetadataList)) {
+ // Remove the existing star-trees
+ LOGGER.info("Removing star-trees from segment: {}",
_segmentMetadata.getName());
+ StarTreeBuilderUtils.removeStarTrees(_indexDir);
+ _segmentMetadata = new SegmentMetadataImpl(_indexDir);
+ } else {
+ // Existing star-trees match the builder configs, no need to
generate the star-trees
+ shouldGenerateStarTree = false;
+ }
+ }
+ // Generate the star-trees if needed
+ if (shouldGenerateStarTree) {
+ // NOTE: Always use OFF_HEAP mode on server side.
+ try (MultipleTreesBuilder builder = new
MultipleTreesBuilder(starTreeBuilderConfigs, _indexDir,
+ MultipleTreesBuilder.BuildMode.OFF_HEAP)) {
+ builder.build();
+ }
+ _segmentMetadata = new SegmentMetadataImpl(_indexDir);
+ }
+ }
+ }
+
+ /**
+ * Remove all the existing inverted index temp files before loading
segments, by looking
+ * for all files in the directory and remove the ones with
'.bitmap.inv.tmp' extension.
+ */
+ private void removeInvertedIndexTempFiles() {
+ File[] directoryListing = _indexDir.listFiles();
+ if (directoryListing == null) {
+ return;
+ }
+ String tempFileExtension =
V1Constants.Indexes.BITMAP_INVERTED_INDEX_FILE_EXTENSION + ".tmp";
+ for (File child : directoryListing) {
+ if (child.getName().endsWith(tempFileExtension)) {
+ FileUtils.deleteQuietly(child);
+ }
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
