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

[GitBox]

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:
us...@infra.apache.org



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