[GitHub] [incubator-pinot] snleee commented on a change in pull request #6975: Merge rollup executor enhancement

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

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



##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
##########
@@ -172,7 +185,7 @@ public static URI getDeleteSegmentHttpUri(String host, int port, String rawTable
       String tableType)
       throws URISyntaxException {
     return new URI(StringUtil.join("/", StringUtils.chomp(HTTP + "://" + host + ":" + port, "/"), OLD_SEGMENT_PATH,
-        rawTableName + "/" + URIUtils.encode(segmentName) + TYPE_DELIMITER + tableType));
+        rawTableName + "/" + URIUtils.encode(segmentName) + "?" + TYPE_DELIMITER + tableType));

Review comment:
       This API looks to be deprecated. Why do we need this change? 

##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java
##########
@@ -185,7 +198,7 @@ public static URI getRetrieveAllSegmentWithTableTypeHttpUri(String host, int por
       String tableType)
       throws URISyntaxException {
     return new URI(StringUtil.join("/", StringUtils.chomp(HTTP + "://" + host + ":" + port, "/"), OLD_SEGMENT_PATH,
-        rawTableName + TYPE_DELIMITER + tableType));
+        rawTableName + "?" + TYPE_DELIMITER + tableType));

Review comment:
       Same here

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java
##########
@@ -46,6 +48,7 @@ private MinionConstants() {
   public static final String RETRY_SCALE_FACTOR_KEY = "retryScaleFactor";
 
   public static final String TABLE_MAX_NUM_TASKS_KEY = "tableMaxNumTasks";
+  public static final String REPLACE_SEGMENTS_KEY = "replaceSegments";

Review comment:
       `enableSegmentsReplacement`?
   

##########
File path: pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/BaseMultipleSegmentsConversionExecutor.java
##########
@@ -138,11 +149,32 @@ protected void postProcess(PinotTaskConfig pinotTaskConfig) {
             taskType + " on table: " + tableNameWithType + ", segments: " + inputSegmentNames + " got cancelled");
       }
 
+      // Update the segment lineage to indicate that the segment replacement is in progress.
+      String lineageEntryId = null;
+      if (replaceSegmentsEnabled) {
+        List<String> segmentsFrom =
+            Arrays.stream(inputSegmentNames.split(",")).map(String::trim).collect(Collectors.toList());
+        List<String> segmentsTo =
+            segmentConversionResults.stream().map(SegmentConversionResult::getSegmentName).collect(Collectors.toList());
+        lineageEntryId = SegmentConversionUtils
+            .startSegmentReplace(tableNameWithType, uploadURL, new StartReplaceSegmentsRequest(segmentsFrom, segmentsTo));
+      }
+
       // Upload the tarred segments
       for (int i = 0; i < numOutputSegments; i++) {
         File convertedTarredSegmentFile = tarredSegmentFiles.get(i);
         String resultSegmentName = segmentConversionResults.get(i).getSegmentName();
 
+        // Set segment ZK metadata custom map modifier into HTTP header to modify the segment ZK metadata
+        SegmentZKMetadataCustomMapModifier segmentZKMetadataCustomMapModifier = getSegmentZKMetadataCustomMapModifier(pinotTaskConfig);
+        Header segmentZKMetadataCustomMapModifierHeader =
+            new BasicHeader(FileUploadDownloadClient.CustomHeaders.SEGMENT_ZK_METADATA_CUSTOM_MAP_MODIFIER,
+                segmentZKMetadataCustomMapModifier.toJsonString());
+
+        List<Header> httpHeaders = new ArrayList<>();
+        httpHeaders.add(segmentZKMetadataCustomMapModifierHeader);
+        httpHeaders.addAll(FileUploadDownloadClient.makeAuthHeader(authToken));

Review comment:
       Can you explain why we need this?




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