deepthi912 commented on code in PR #19349:
URL: https://github.com/apache/pinot/pull/19349#discussion_r3901859428
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java:
##########
@@ -2025,6 +2030,245 @@ public void testStarTreeCreationWithDictionaryChanges()
}
}
+ /// A star-tree dimension column that is moved to 'noDictionaryColumns'
without the star-tree being rebuilt leaves
+ /// the star-tree unreadable: its dimension forward index stores dictionary
ids in a fixed-bit encoding whose width
+ /// is read from the main column metadata, which is now raw. The stale
star-tree must be dropped so the segment
+ /// stays loadable, even when dynamic star-tree creation is disabled.
+ @Test
+ public void testStarTreeDimensionConvertedToNoDictionary()
+ throws Exception {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+ .addMetric("longCol", DataType.LONG)
+ .build();
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("stringCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ buildStarTreeTestSegment(tableConfig, schema);
+
+ // Drift the config: the star-tree dimension is moved to
noDictionaryColumns and the star-tree config is dropped,
+ // while dynamic star-tree creation stays disabled.
+ indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+ indexingConfig.setStarTreeIndexConfigs(null);
+ indexingConfig.setEnableDynamicStarTreeCreation(false);
+ IndexLoadingConfig indexLoadingConfig = new
IndexLoadingConfig(tableConfig, schema);
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentPreProcessor processor = new
SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+ assertTrue(processor.needProcess());
+ processor.process(SEGMENT_OPERATIONS_THROTTLER);
+ }
+ assertSegmentLoadsWithoutStarTree(indexLoadingConfig);
+
+ // The stale star-tree is gone, so there is nothing left to process
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentPreProcessor processor = new
SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+ assertFalse(processor.needProcess());
+ }
+ }
+
+ /// Same drift as [#testStarTreeDimensionConvertedToNoDictionary()], but the
dict-to-raw conversion has already been
+ /// persisted by an earlier pre-processing round, so the segment on disk is
already inconsistent and nothing else
+ /// needs updating. Pre-processing must still detect and repair it.
+ @Test
+ public void testStarTreeDimensionAlreadyConvertedToNoDictionary()
+ throws Exception {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+ .addMetric("longCol", DataType.LONG)
+ .build();
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("stringCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ buildStarTreeTestSegment(tableConfig, schema);
+
+ // Convert the dimension column to raw while leaving the star-tree in
place, reproducing the state an earlier
+ // pre-processing round leaves behind.
+ indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+ IndexLoadingConfig indexLoadingConfig = new
IndexLoadingConfig(tableConfig, schema);
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+ new ForwardIndexHandler(segmentDirectory,
indexLoadingConfig).updateIndices(segmentDirectory.createWriter());
+ }
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+
assertFalse(segmentDirectory.getSegmentMetadata().getColumnMetadataFor("stringCol").hasDictionary());
+
assertNotNull(segmentDirectory.getSegmentMetadata().getStarTreeV2MetadataList());
+ }
+
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentPreProcessor processor = new
SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+ assertTrue(processor.needProcess());
+ processor.process(SEGMENT_OPERATIONS_THROTTLER);
+ }
+ assertSegmentLoadsWithoutStarTree(indexLoadingConfig);
+ }
+
+ /// The loader must not fail the whole segment over a stale star-tree even
when pre-processing never gets a chance to
+ /// repair it, e.g. because it is skipped for the table.
+ @Test
+ public void testStarTreeDimensionConvertedToNoDictionaryWithoutPreprocess()
+ throws Exception {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+ .addMetric("longCol", DataType.LONG)
+ .build();
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("stringCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ buildStarTreeTestSegment(tableConfig, schema);
+
+ indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+ IndexLoadingConfig indexLoadingConfig = new
IndexLoadingConfig(tableConfig, schema);
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+ new ForwardIndexHandler(segmentDirectory,
indexLoadingConfig).updateIndices(segmentDirectory.createWriter());
+ }
+
+ // The stale star-tree is still in the segment, but it must be skipped
rather than fail the load
+ ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR,
indexLoadingConfig, false);
+ try {
+ assertEquals(segment.getSegmentMetadata().getTotalDocs(), 5);
+ assertTrue(segment.getStarTrees() == null ||
segment.getStarTrees().isEmpty());
+ } finally {
+ segment.destroy();
+ }
+ }
+
+ /// With dynamic star-tree creation enabled, the stale star-tree is not just
dropped but rebuilt from the current
+ /// config, which no longer splits on the re-encoded column.
+ @Test
+ public void testStarTreeDimensionConvertedToNoDictionaryWithDynamicCreation()
+ throws Exception {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+ .addSingleValueDimension("intCol", DataType.INT)
+ .addMetric("longCol", DataType.LONG)
+ .build();
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("stringCol", "intCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ buildStarTreeTestSegment(tableConfig, schema);
+
+ // 'stringCol' becomes raw and drops out of the split order, and the
star-tree is rebuilt on 'intCol' alone
+ indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("intCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ indexingConfig.setEnableDynamicStarTreeCreation(true);
+ IndexLoadingConfig indexLoadingConfig = new
IndexLoadingConfig(tableConfig, schema);
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentPreProcessor processor = new
SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+ assertTrue(processor.needProcess());
+ processor.process(SEGMENT_OPERATIONS_THROTTLER);
+ }
+
+ ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR,
indexLoadingConfig, false);
+ try {
+ List<StarTreeV2> starTrees = segment.getStarTrees();
+ assertNotNull(starTrees);
+ assertEquals(starTrees.size(), 1);
+ assertEquals(starTrees.get(0).getMetadata().getDimensionsSplitOrder(),
List.of("intCol"));
+ } finally {
+ segment.destroy();
+ }
+ }
+
+ /// Apache Pinot PR #19153 added star-tree support for dimensions stored as
a `RAW` forward index with a separated
+ /// dictionary. Such a column still has a dictionary, so its star-tree stays
readable and must NOT be treated as
+ /// stale: pre-processing has to flip the forward index to raw, keep the
dictionary, and leave the star-tree alone.
+ @Test
+ public void testStarTreeDimensionConvertedToRawWithSeparatedDictionary()
+ throws Exception {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+ Schema schema = new
Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+ .addMetric("longCol", DataType.LONG)
+ .build();
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ indexingConfig.setStarTreeIndexConfigs(
+ List.of(new StarTreeIndexConfig(List.of("stringCol"), null,
List.of("SUM__longCol"), null, 1000)));
+ buildStarTreeTestSegment(tableConfig, schema);
+
+ // Keep the star-tree config, but store the dimension as RAW forward index
with the dictionary kept alongside
+ ObjectNode indexes = JsonUtils.newObjectNode();
+ ObjectNode forwardConfig = JsonUtils.newObjectNode();
+ forwardConfig.put("encodingType", "RAW");
+ indexes.set("forward", forwardConfig);
+ ObjectNode dictionaryConfig = JsonUtils.newObjectNode();
+ dictionaryConfig.put("disabled", false);
+ indexes.set("dictionary", dictionaryConfig);
+ tableConfig.setFieldConfigList(List.of(
+ new
FieldConfig.Builder("stringCol").withEncodingType(FieldConfig.EncodingType.RAW)
+ .withIndexes(indexes)
+ .build()));
+ indexingConfig.setEnableDynamicStarTreeCreation(false);
+ IndexLoadingConfig indexLoadingConfig = new
IndexLoadingConfig(tableConfig, schema);
+
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentPreProcessor processor = new
SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+ processor.process(SEGMENT_OPERATIONS_THROTTLER);
Review Comment:
Agree don't go for process directly in tests we might miss on
`skipSegmentPreProcess` and other checks we do, do check for needPreProcess or
best efforts check for `skipSegmentPreProcess`.
Check: `needPreprocess(SegmentDirectory segmentDirectory, IndexLoadingConfig
indexLoadingConfig)` method
--
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]
