gortiz commented on code in PR #10184:
URL: https://github.com/apache/pinot/pull/10184#discussion_r1153313825
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java:
##########
@@ -104,20 +104,31 @@ public void process()
}
// Update single-column indices, like inverted index, json index etc.
- IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
List<IndexHandler> indexHandlers = new ArrayList<>();
- for (ColumnIndexType type : ColumnIndexType.values()) {
- IndexHandler handler =
- IndexHandlerFactory.getIndexHandler(type, _segmentDirectory,
_indexLoadingConfig, _schema);
- indexHandlers.add(handler);
- // TODO: Find a way to ensure ForwardIndexHandler is always executed
before other handlers instead of
- // relying on enum ordering.
- handler.updateIndices(segmentWriter, indexCreatorProvider);
- // ForwardIndexHandler may modify the segment metadata while rewriting
forward index to create / remove a
- // dictionary. Other IndexHandler classes may modify the segment
metadata while creating a temporary forward
- // index to generate their respective indexes from if the forward
index was disabled. This new metadata is
- // needed to construct other indexes like RangeIndex.
- _segmentMetadata = _segmentDirectory.getSegmentMetadata();
+
+ // We cannot just create all the index handlers in a random order.
+ // Specifically, ForwardIndexHandler needs to be executed first. This is
because it modifies the segment metadata
+ // while rewriting forward index to create a dictionary. Some other
handlers (like the range one) assume that
+ // metadata was already been modified by ForwardIndexHandler.
+ IndexHandler forwardHandler = createHandler(StandardIndexes.forward());
+ indexHandlers.add(forwardHandler);
+ forwardHandler.updateIndices(segmentWriter);
+
+ // Now that ForwardIndexHandler.updateIndeces has been updated, we can
run all other indexes in any order
+
+ _segmentMetadata = new SegmentMetadataImpl(indexDir);
+ _segmentDirectory.reloadMetadata();
Review Comment:
TBH I'm not completely sure. What we did before after each handler is:
```
_segmentMetadata = _segmentDirectory.getSegmentMetadata();
```
It may be sufficient to do that here too.
--
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]
