[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10089: Reload conditional skip

[GitBox]

Jackie-Jiang commented on code in PR #10089:
URL: https://github.com/apache/pinot/pull/10089#discussion_r1072956009


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -355,13 +355,33 @@ public void reloadSegment(String segmentName, 
IndexLoadingConfig indexLoadingCon
     
indexLoadingConfig.setInstanceTierConfigs(_tableDataManagerConfig.getInstanceTierConfigs());
     File indexDir = getSegmentDataDir(segmentName, segmentTier, 
indexLoadingConfig.getTableConfig());
     try {
+      boolean shouldDownload = forceDownload || !hasSameCRC(zkMetadata, 
localMetadata);
+
+      if (!shouldDownload && (zkMetadata.getTier().equals(segmentTier))) {
+        // We should first try to reuse existing segment directory
+        SegmentDirectory segmentDirectory =
+            initSegmentDirectory(segmentName, 
String.valueOf(zkMetadata.getCrc()), indexLoadingConfig);
+        boolean needReprocess = 
ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig, 
schema);
+        if (!needReprocess) {
+          LOGGER.info("Reloading segment : {} of table : {} using existing 
segment directory as no reprocessing needed",

Review Comment:
   (minor)
   ```suggestion
             LOGGER.info("Reloading segment: {} of table: {} using existing 
segment directory as no preprocessing is needed",
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -355,13 +355,33 @@ public void reloadSegment(String segmentName, 
IndexLoadingConfig indexLoadingCon
     
indexLoadingConfig.setInstanceTierConfigs(_tableDataManagerConfig.getInstanceTierConfigs());
     File indexDir = getSegmentDataDir(segmentName, segmentTier, 
indexLoadingConfig.getTableConfig());
     try {
+      boolean shouldDownload = forceDownload || !hasSameCRC(zkMetadata, 
localMetadata);
+
+      if (!shouldDownload && (zkMetadata.getTier().equals(segmentTier))) {
+        // We should first try to reuse existing segment directory
+        SegmentDirectory segmentDirectory =
+            initSegmentDirectory(segmentName, 
String.valueOf(zkMetadata.getCrc()), indexLoadingConfig);
+        boolean needReprocess = 
ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig, 
schema);

Review Comment:
   (minor)
   ```suggestion
           boolean needPreprocess = 
ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig, 
schema);
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -355,13 +355,33 @@ public void reloadSegment(String segmentName, 
IndexLoadingConfig indexLoadingCon
     
indexLoadingConfig.setInstanceTierConfigs(_tableDataManagerConfig.getInstanceTierConfigs());
     File indexDir = getSegmentDataDir(segmentName, segmentTier, 
indexLoadingConfig.getTableConfig());
     try {
+      boolean shouldDownload = forceDownload || !hasSameCRC(zkMetadata, 
localMetadata);
+
+      if (!shouldDownload && (zkMetadata.getTier().equals(segmentTier))) {
+        // We should first try to reuse existing segment directory
+        SegmentDirectory segmentDirectory =
+            initSegmentDirectory(segmentName, 
String.valueOf(zkMetadata.getCrc()), indexLoadingConfig);
+        boolean needReprocess = 
ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig, 
schema);
+        if (!needReprocess) {
+          LOGGER.info("Reloading segment : {} of table : {} using existing 
segment directory as no reprocessing needed",
+              segmentName, _tableNameWithType);
+          // No reprocessing needed, reuse the same segment
+          ImmutableSegment segment = 
ImmutableSegmentLoader.load(segmentDirectory, indexLoadingConfig, schema);
+          addSegment(segment);
+          return;
+        } else {
+          LOGGER.info("Segment : {} of table : {} requires reprocessing before 
reloading", segmentName,

Review Comment:
   (minor)
   ```suggestion
             LOGGER.info("Segment: {} of table: {} requires preprocessing 
before reloading", segmentName,
   ```



-- 
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