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 2022/06/15 14:49:33 UTC
[GitHub] [pinot] walterddr commented on a diff in pull request #8823: Allow moveToFinalLocation in METADATA push based on config
walterddr commented on code in PR #8823:
URL: https://github.com/apache/pinot/pull/8823#discussion_r898080999
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SegmentPushUtils.java:
##########
@@ -264,6 +264,10 @@ public static void sendSegmentUriAndMetadata(SegmentGenerationJobSpec spec, Pino
headers.add(new BasicHeader(FileUploadDownloadClient.CustomHeaders.DOWNLOAD_URI, segmentUriPath));
headers.add(new BasicHeader(FileUploadDownloadClient.CustomHeaders.UPLOAD_TYPE,
FileUploadDownloadClient.FileUploadType.METADATA.toString()));
+ if (spec.getPushJobSpec() != null) {
+ headers.add(new BasicHeader(FileUploadDownloadClient.CustomHeaders.COPY_SEGMENT_TO_DEEP_STORE,
+ String.valueOf(spec.getPushJobSpec().getMoveToDeepStoreForMetadataPush())));
Review Comment:
I guess I was a bit confused with the PushJobSpec definition name "move" and the CustomHeader field "copy". so it actually resembles a fils system "copy" but not a "move" correct? maybe renaming to avoid confusion?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java:
##########
@@ -260,14 +264,23 @@ private SuccessResponse uploadSegment(@Nullable String tableName, TableType tabl
"Segment metadata file (as multipart/form-data) is required for METADATA upload mode",
Response.Status.BAD_REQUEST);
}
- if (StringUtils.isEmpty(downloadURI)) {
- throw new ControllerApplicationException(LOGGER, "Download URI is required for METADATA upload mode",
+ if (StringUtils.isEmpty(sourceDownloadURIStr)) {
+ throw new ControllerApplicationException(LOGGER,
+ "Source download URI is required in header field 'DOWNLOAD_URI' for METADATA upload mode",
Response.Status.BAD_REQUEST);
}
- moveSegmentToFinalLocation = false;
+ // override copySegmentToFinalLocation if override provided in headers:COPY_SEGMENT_TO_DEEP_STORE
+ // else set to false for backward compatibility
+ String copySegmentToDeepStore =
+ extractHttpHeader(headers, FileUploadDownloadClient.CustomHeaders.COPY_SEGMENT_TO_DEEP_STORE);
+ if (copySegmentToDeepStore != null) {
+ copySegmentToFinalLocation = Boolean.parseBoolean(copySegmentToDeepStore);
+ } else {
+ copySegmentToFinalLocation = false;
+ }
Review Comment:
```suggestion
copySegmentToFinalLocation = Boolean.parseBoolean(copySegmentToDeepStore);
```
should do the trick. null is mapped to false.
--
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-unsubscribe@pinot.apache.org
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