You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/26 01:48:29 UTC

[GitHub] [incubator-pinot] snleee commented on a change in pull request #6976: Fix deep store directory structure

snleee commented on a change in pull request #6976:
URL: https://github.com/apache/incubator-pinot/pull/6976#discussion_r639344529



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##########
@@ -386,11 +386,11 @@ private SegmentMetadata getSegmentMetadata(File tempDecryptedFile, File tempSegm
   }
 
   private void completeZkOperations(boolean enableParallelPushProtection, HttpHeaders headers, File uploadedSegmentFile,
-      String tableNameWithType, SegmentMetadata segmentMetadata, String segmentName, String zkDownloadURI,
-      boolean moveSegmentToFinalLocation, String crypter)
+      String rawTableName, String tableNameWithType, SegmentMetadata segmentMetadata, String segmentName,
+      String zkDownloadURI, boolean moveSegmentToFinalLocation, String crypter)
       throws Exception {
     URI finalSegmentLocationURI = URIUtils
-        .getUri(ControllerFilePathProvider.getInstance().getDataDirURI().toString(), tableNameWithType,
+        .getUri(ControllerFilePathProvider.getInstance().getDataDirURI().toString(), rawTableName,
             URIUtils.encode(segmentName));
     ZKOperator zkOperator = new ZKOperator(_pinotHelixResourceManager, _controllerConf, _controllerMetrics);
     zkOperator.completeSegmentOperations(tableNameWithType, segmentMetadata, finalSegmentLocationURI, uploadedSegmentFile,

Review comment:
       If you check the previous PR for `completeSegmentOperations()`, there are many parts where we change `rawTableName -> tableNameWithType`. Those changes may need to be reverted as well?
   
   e.g.
   https://github.com/apache/incubator-pinot/pull/6567/files#diff-ab01aaed27b0a00398747217dea97c6ae4e13a1e51682c93da43375e87d15761L73
   
   Or, is the fix in the PR sufficient? @chenboat probably has more context around this.

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##########
@@ -386,11 +386,11 @@ private SegmentMetadata getSegmentMetadata(File tempDecryptedFile, File tempSegm
   }
 
   private void completeZkOperations(boolean enableParallelPushProtection, HttpHeaders headers, File uploadedSegmentFile,
-      String tableNameWithType, SegmentMetadata segmentMetadata, String segmentName, String zkDownloadURI,
-      boolean moveSegmentToFinalLocation, String crypter)
+      String rawTableName, String tableNameWithType, SegmentMetadata segmentMetadata, String segmentName,

Review comment:
       We can compute `rawTableName` from `tableNameWithType` by `TableNameBuilder.extractRawTableName(tableNameWithType)`




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

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