Jackie-Jiang commented on a change in pull request #7286:
URL: https://github.com/apache/pinot/pull/7286#discussion_r695092974
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
##########
@@ -385,10 +385,12 @@ public void removeIndex(String columnName,
ColumnIndexType indexType) {
@Override
public Set<String> getColumnsWithIndex(ColumnIndexType type) {
+ // TEXT_INDEX is not tracked via _columnEntries, so need to call
Review comment:
Specialize the handling for `TEXT` and use the current logic for other
index types for efficiency
##########
File path: pinot-segment-local/src/test/resources/data/log4j2.xml
##########
@@ -29,7 +29,7 @@
</Appenders>
<Loggers>
<AsyncRoot level="warn" additivity="false">
- <AppenderRef ref="console"/>
+ <AppenderRef ref="console" />
Review comment:
revert
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -57,25 +57,40 @@ private ImmutableSegmentLoader() {
private static final Logger LOGGER =
LoggerFactory.getLogger(ImmutableSegmentLoader.class);
/**
- * For tests only.
+ * load with empty schema and IndexLoadingConfig is used to get a handler of
the segment for
+ * its metadata and any other existing info inside the segment folder. This
method is used by
+ * MultiTreeBuilder (with empty schema and IndexLoadingConfig to avoid
recursion) and tools.
+ * No need to cleanup indices while calling this load method.
*/
public static ImmutableSegment load(File indexDir, ReadMode readMode)
throws Exception {
IndexLoadingConfig defaultIndexLoadingConfig = new IndexLoadingConfig();
defaultIndexLoadingConfig.setReadMode(readMode);
- return load(indexDir, defaultIndexLoadingConfig, null);
+ return load(indexDir, defaultIndexLoadingConfig, null, false);
}
/**
- * For tests only.
+ * load with empty schema but a specified IndexLoadingConfig (mostly empty)
is also used
+ * to get a handler of the current segment, e.g. used in RawIndexConverter
and test cases.
+ * No need to cleanup indices while calling this load method.
*/
public static ImmutableSegment load(File indexDir, IndexLoadingConfig
indexLoadingConfig)
throws Exception {
- return load(indexDir, indexLoadingConfig, null);
+ return load(indexDir, indexLoadingConfig, null, false);
}
+ /**
+ * load with specified schema and IndexLoadingConfig and clean up obsolete
indices
+ * according to the IndexLoadingConfig.
+ */
public static ImmutableSegment load(File indexDir, IndexLoadingConfig
indexLoadingConfig, @Nullable Schema schema)
throws Exception {
+ return load(indexDir, indexLoadingConfig, schema, true);
+ }
+
+ private static ImmutableSegment load(File indexDir, IndexLoadingConfig
indexLoadingConfig, @Nullable Schema schema,
+ boolean cleanupIndices)
Review comment:
I think we should make segment pre-processing optional instead of making
index cleanup optional. When we do the segment pre-processing, we want to make
the segment align with the index loading config and schema.
--
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]
