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-unsubscr...@pinot.apache.org
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
