klsince commented on code in PR #10089: URL: https://github.com/apache/pinot/pull/10089#discussion_r1073866912
########## pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java: ########## @@ -403,6 +419,16 @@ public void reloadSegment(String segmentName, IndexLoadingConfig indexLoadingCon } } + public boolean canReuseExistingDirectoryForReload(SegmentZKMetadata segmentZKMetadata, + String currentSegmentTier, SegmentDirectory segmentDirectory, IndexLoadingConfig indexLoadingConfig, + Schema schema) + throws Exception { + boolean tierMigrationNeeded = indexLoadingConfig.getSegmentDirectoryLoader() + .equalsIgnoreCase(TierBasedSegmentDirectoryLoader.DIRECTORY_LOADER_NAME) && !StringUtils.equals( + segmentZKMetadata.getTier(), currentSegmentTier); + return !tierMigrationNeeded && !ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig, schema); Review Comment: curious why need to explicitly check if loader name is `tierBased`? With this check, it seems like other custom loaders that can do tier migration would be directed to reuse existing directory, when it's supposed to handle tier migration. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org