yashmayya commented on code in PR #16953:
URL: https://github.com/apache/pinot/pull/16953#discussion_r2403629200
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java:
##########
@@ -70,6 +73,23 @@ public boolean needUpdateIndices(SegmentDirectory.Reader
segmentReader) {
if (!columnsToAddIdx.remove(column)) {
LOGGER.info("Need to remove existing H3 index from segment: {},
column: {}", segmentName, column);
return true;
+ } else {
+ // Index already exists, check for change in resolution config
+ short newResolution =
_h3Configs.get(column).getResolution().serialize();
+ short oldResolution;
+ try (H3IndexReader indexReader = new ImmutableH3IndexReader(
+ segmentReader.getIndexFor(column, StandardIndexes.h3()))) {
+ oldResolution = indexReader.getH3IndexResolution().serialize();
+ } catch (IOException e) {
+ LOGGER.warn("Failed to read existing H3 index for segment: {},
column: {}", segmentName, column, e);
+ continue;
+ }
+ if (newResolution != oldResolution) {
Review Comment:
I'm not sure that helps much, since `IndexHandler::updateIndices` would need
to have similar logic anyway to determine the columns for which the index needs
to be rebuilt. Unless you're proposing moving the delete part of the rebuild
into the first check for whether processing is needed - but that will be a much
larger refactor since the method is used to determine whether segments need to
be reloaded or refreshed as well.
--
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]
