[GitHub] [pinot] npawar commented on a diff in pull request #9306: TierBasedSegmentDirectoryLoader to keep segments in multi-datadir

[GitBox <gi...@apache.org>]

npawar commented on code in PR #9306:
URL: https://github.com/apache/pinot/pull/9306#discussion_r967914605


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -642,44 +675,44 @@ private boolean tryLoadExistingSegment(String segmentName, IndexLoadingConfig in
         // Close the stale SegmentDirectory object and recreate it with reprocessed segment.
         closeSegmentDirectoryQuietly(segmentDirectory);
         ImmutableSegmentLoader.preprocess(indexDir, indexLoadingConfig, schema);
-        segmentDirectory =
-            initSegmentDirectory(segmentName, String.valueOf(zkMetadata.getCrc()), indexLoadingConfig, schema);
+        segmentDirectory = initSegmentDirectory(segmentName, String.valueOf(zkMetadata.getCrc()), zkMetadata.getTier(),
+            indexLoadingConfig, schema);
       }
       ImmutableSegment segment = ImmutableSegmentLoader.load(segmentDirectory, indexLoadingConfig, schema);
       addSegment(segment);
-      LOGGER.info("Loaded existing segment: {} of table: {} with crc: {}", segmentName, _tableNameWithType,
-          zkMetadata.getCrc());
+      LOGGER.info("Loaded existing segment: {} of table: {} with crc: {} on tier: {}", segmentName, _tableNameWithType,
+          zkMetadata.getCrc(), zkMetadata.getTier());
       return true;
     } catch (Exception e) {
-      LOGGER.error("Failed to load existing segment: {} of table: {} with crc: {}", segmentName, _tableNameWithType,
-          zkMetadata.getCrc(), e);
+      LOGGER.error("Failed to load existing segment: {} of table: {} with crc: {} on tier: {}", segmentName,
+          _tableNameWithType, zkMetadata.getCrc(), zkMetadata.getTier(), e);
       closeSegmentDirectoryQuietly(segmentDirectory);
       return false;
     }
   }
 
-  private SegmentDirectory tryInitSegmentDirectory(String segmentName, String segmentCrc,
+  private SegmentDirectory tryInitSegmentDirectory(String segmentName, String segmentCrc, String segmentTier,
       IndexLoadingConfig indexLoadingConfig, Schema schema) {
     try {
-      return initSegmentDirectory(segmentName, segmentCrc, indexLoadingConfig, schema);
+      return initSegmentDirectory(segmentName, segmentCrc, segmentTier, indexLoadingConfig, schema);
     } catch (Exception e) {
-      LOGGER.warn("Failed to initialize SegmentDirectory for segment: {} of table: {} with error: {}", segmentName,
-          _tableNameWithType, e.getMessage());
+      LOGGER.warn("Failed to initialize SegmentDirectory for segment: {} of table: {} on tier: {} with error: {}",
+          segmentName, _tableNameWithType, segmentTier, e.getMessage());
       return null;
     }
   }
 
-  private SegmentDirectory initSegmentDirectory(String segmentName, String segmentCrc,
+  private SegmentDirectory initSegmentDirectory(String segmentName, String segmentCrc, String segmentTier,
       IndexLoadingConfig indexLoadingConfig, Schema schema)
       throws Exception {
     SegmentDirectoryLoaderContext loaderContext =
         new SegmentDirectoryLoaderContext.Builder().setTableConfig(indexLoadingConfig.getTableConfig())
-            .setSchema(schema).setInstanceId(indexLoadingConfig.getInstanceId()).setSegmentName(segmentName)
-            .setSegmentCrc(segmentCrc).setSegmentDirectoryConfigs(indexLoadingConfig.getSegmentDirectoryConfigs())
-            .build();
+            .setSchema(schema).setInstanceId(indexLoadingConfig.getInstanceId()).setTableDataDir(_tableDataDir)
+            .setSegmentName(segmentName).setSegmentCrc(segmentCrc).setSegmentTier(segmentTier)
+            .setSegmentDirectoryConfigs(indexLoadingConfig.getSegmentDirectoryConfigs()).build();
     SegmentDirectoryLoader segmentDirectoryLoader =
         SegmentDirectoryLoaderRegistry.getSegmentDirectoryLoader(indexLoadingConfig.getSegmentDirectoryLoader());
-    File indexDir = getSegmentDataDir(segmentName);
+    File indexDir = getSegmentDataDir(segmentName, segmentTier, indexLoadingConfig);
     return segmentDirectoryLoader.load(indexDir.toURI(), loaderContext);

Review Comment:
   minor suggestion: in that case, please add current/target or src/dest prefix to every segmentTier and indexDir variable



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org