[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6567: Enable segment upload to a realtime table

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

mcvsubbu commented on a change in pull request #6567:
URL: https://github.com/apache/incubator-pinot/pull/6567#discussion_r595569771



##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##########
@@ -358,7 +358,7 @@
   public static class Segment {
     public static class Realtime {
       public enum Status {
-        IN_PROGRESS, DONE
+        IN_PROGRESS, DONE, UPLOAD

Review comment:
       ```suggestion
           IN_PROGRESS, DONE, UPLOADED
   ```
   Can you please add some comments on what each of these mean? With the addition of the new value here, we need to do that.
   
   IN_PROGRESS means the segment is being consumed
   DONE means the segment is completed, and uploaded by pinot-server.
   UPLOADED means the segment is uploaded by some external agent (?)

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##########
@@ -245,21 +245,28 @@ private SuccessResponse uploadSegment(@Nullable String tableName, FormDataMultiP
         LOGGER.info("Uploading a segment {} to table: {}, push type {}, (Derived from segment metadata)", segmentName, tableName, uploadType);
       }
 
-      String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(rawTableName);
+      String tableNameWithType;
+      if (_pinotHelixResourceManager.isUpsertTable(rawTableName)) {

Review comment:
       This logic seems a little strange. Does this mean that we cannot upload segment to the offline part of an upsert table ? Or that offline part does not exist for an upsert table? What if we introduce this part later?
   Instead, I suggest that we include the type query-part as well in the url. If the type is not present, we assume it is OFFLINE. If it is present, we treat it accordingly, but throw an exception if it is realtime type but upsert is not turned on




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