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

Reply via email to