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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
