xiangfu0 commented on code in PR #18920:
URL: https://github.com/apache/pinot/pull/18920#discussion_r3566274631
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/JsonIndexHandler.java:
##########
@@ -65,12 +71,23 @@ public boolean needUpdateIndices(SegmentDirectory.Reader
segmentReader) {
String segmentName = _segmentDirectory.getSegmentMetadata().getName();
Set<String> columnsToAddIdx = new HashSet<>(_jsonIndexConfigs.keySet());
Set<String> existingColumns =
segmentReader.toSegmentDirectory().getColumnsWithIndex(StandardIndexes.json());
- // Check if any existing index need to be removed.
+ // Load metadata properties once to avoid repeated disk reads when
checking multiple columns.
+ PropertiesConfiguration properties = loadMetadataProperties();
+ // Check if any existing index need to be removed or if the config changed.
for (String column : existingColumns) {
if (!columnsToAddIdx.remove(column)) {
LOGGER.info("Need to remove existing json index from segment: {},
column: {}", segmentName, column);
return true;
}
+ // Column exists in both existing indexes and config; check if config
changed.
+ // When properties != null but storedConfig is null the index predates
config persistence (legacy segment):
+ // treat as a one-time rebuild to backfill the stored config and catch
any config drift.
+ JsonIndexConfig currentConfig = _jsonIndexConfigs.get(column);
+ JsonIndexConfig storedConfig = readStoredJsonIndexConfig(column,
properties);
+ if (properties != null && (storedConfig == null ||
!storedConfig.equals(currentConfig))) {
Review Comment:
Treating `storedConfig == null` as a rebuild forces every pre-metadata JSON
index through `createJsonIndexForColumn()`. Existing segments can have a JSON
index on a forward-index-disabled column without the dictionary/inverted pair
required by `createForwardIndexIfNeeded()` because
`JsonIndexType.requiresDictionary()` is false. Those segments loaded before,
but this path now removes the existing JSON index and then fails reload even
when table config did not change. Please only backfill this metadata when the
forward index is present/reconstructable, or preserve the existing index and
require refresh/backfill for this legacy case.
--
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]
