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 2021/03/06 00:36:13 UTC

[GitHub] [incubator-pinot] liuchang0520 opened a new pull request #6653: add uploadLLCSegment endpoint in TableResource

liuchang0520 opened a new pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653


   ## Description
   To complete [deep store bypass for realtime segment completion](https://cwiki.apache.org/confluence/display/PINOT/By-passing+deep-store+requirement+for+Realtime+segment+completion), we need to create an endpoint to upload LLC segment to segment store.  
   
   Will create another pr for controller periodic task: **RealtimeSegmentValidationManager** to call this endpoint for LLC segment without segment store copy. 
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? No. 
   
   Does this PR fix a zero-downtime upgrade introduced earlier? No.
   
   Does this PR otherwise need attention when creating release notes? No.
   
   ## Documentation
   This change is proposed by: [By-passing deep-store requirement for Realtime segment completion](https://cwiki.apache.org/confluence/display/PINOT/By-passing+deep-store+requirement+for+Realtime+segment+completion) 
   


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

[GitHub] [incubator-pinot] liuchang0520 closed pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 closed pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653


   


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

[GitHub] [incubator-pinot] chenboat commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r593335247



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url
+  @POST
+  @Path("/segments/{realtimeTableName}/{segmentName}")

Review comment:
       Add _upload_ to the rest path so it is clear what this api is doing.




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

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r590583002



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url

Review comment:
       Also mention here that the invocation of this API may cause query performance to suffer, since we tar up the segment to upload it. 
   You may also want to add a comment that upload failure is ok here, since the caller may choose to retry later




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

[GitHub] [incubator-pinot] snleee edited a comment on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
snleee edited a comment on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791833134


   Hi @liuchang0520, this feature is already being worked on at https://github.com/apache/incubator-pinot/pull/6567. 
   
   Are you aware of 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

[GitHub] [incubator-pinot] codecov-io commented on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791825604


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=h1) Report
   > Merging [#6653](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=desc) (01f78a2) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `0.61%`.
   > The diff coverage is `62.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6653/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6653      +/-   ##
   ==========================================
   - Coverage   66.44%   65.83%   -0.62%     
   ==========================================
     Files        1075     1360     +285     
     Lines       54773    66562   +11789     
     Branches     8168     9705    +1537     
   ==========================================
   + Hits        36396    43818    +7422     
   - Misses      15700    19619    +3919     
   - Partials     2677     3125     +448     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `65.83% <62.64%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
   | [...pinot/broker/api/resources/PinotClientRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdENsaWVudFJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (-27.28%)` | :arrow_down: |
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <ø> (+9.52%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <ø> (-13.29%)` | :arrow_down: |
   | [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `82.85% <ø> (+10.12%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
   | ... and [1243 more](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=footer). Last update [5137025...01f78a2](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791825604


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=h1) Report
   > Merging [#6653](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=desc) (01f78a2) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **increase** coverage by `7.30%`.
   > The diff coverage is `80.37%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6653/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6653      +/-   ##
   ==========================================
   + Coverage   66.44%   73.75%   +7.30%     
   ==========================================
     Files        1075     1360     +285     
     Lines       54773    66562   +11789     
     Branches     8168     9705    +1537     
   ==========================================
   + Hits        36396    49094   +12698     
   + Misses      15700    14249    -1451     
   - Partials     2677     3219     +542     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `43.85% <42.85%> (?)` | |
   | unittests | `65.83% <62.64%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `73.33% <ø> (+16.19%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <ø> (-4.40%)` | :arrow_down: |
   | [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `88.57% <ø> (+15.84%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
   | [...va/org/apache/pinot/client/ExternalViewReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4dGVybmFsVmlld1JlYWRlci5qYXZh) | `81.69% <ø> (+31.69%)` | :arrow_up: |
   | [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `56.36% <ø> (-5.64%)` | :arrow_down: |
   | ... and [1204 more](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=footer). Last update [5137025...01f78a2](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r590579757



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url

Review comment:
       Add a pointer to the design section here, so that there is some context:
   https://cwiki.apache.org/confluence/display/PINOT/By-passing+deep-store+requirement+for+Realtime+segment+completion#BypassingdeepstorerequirementforRealtimesegmentcompletion-Failurecasesandhandling
   Mention in comments that this API is used when segment store is unavailable during llc completion. 




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

[GitHub] [incubator-pinot] liuchang0520 commented on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791840974


   > Hi @liuchang0520, this feature is already being worked on at #6567.
   > 
   > Are you aware of 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

[GitHub] [incubator-pinot] liuchang0520 commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r598472566



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -24,36 +24,34 @@
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import java.io.File;
+import java.net.URI;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import javax.inject.Inject;
-import javax.ws.rs.DefaultValue;
-import javax.ws.rs.Encoded;
-import javax.ws.rs.GET;
-import javax.ws.rs.Path;
-import javax.ws.rs.PathParam;
-import javax.ws.rs.Produces;
-import javax.ws.rs.QueryParam;
-import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.*;

Review comment:
       Thanks for the pointer.




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

[GitHub] [incubator-pinot] liuchang0520 commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r598476535



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url

Review comment:
       Thanks for the pointer @mcvsubbu .  Comments updated.




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

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r590583556



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url
+  @POST
+  @Path("/segments/{realtimeTableName}/{segmentName}")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Upload a low level consumer segment to segment store and return the segment download url", notes = "Upload a low level consumer segment to segment store and return the segment download url")
+  @ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error", response = ErrorInfo.class), @ApiResponse(code = 404, message = "Table or segment not found", response = ErrorInfo.class), @ApiResponse(code = 400, message = "Bad request", response = ErrorInfo.class)})
+  public String uploadLLCSegment(
+          @ApiParam(value = "Name of the REALTIME table", required = true) @PathParam("realtimeTableName") String realtimeTableName,
+          @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") String segmentName) throws Exception {
+    LOGGER.info("Received a request to upload low level consumer segment {} for table {}", segmentName, realtimeTableName);
+
+    // Check it's realtime table
+    TableType tableType = TableNameBuilder.getTableTypeFromTableName(realtimeTableName);
+    if (TableType.OFFLINE == tableType) {
+      throw new WebApplicationException(
+              String.format("Cannot upload low level consumer segment for OFFLINE table: %s", realtimeTableName),
+              Response.Status.BAD_REQUEST);
+    }
+
+    // Check the segment is low level consumer segment
+    if (!LLCSegmentName.isLowLevelConsumerSegmentName(segmentName)) {
+      throw new WebApplicationException(
+              String.format("Segment %s is not a low level consumer segment", segmentName),
+              Response.Status.BAD_REQUEST);
+    }
+
+    String tableNameWithType = TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(realtimeTableName);
+    TableDataManager tableDataManager = checkGetTableDataManager(tableNameWithType);
+    SegmentDataManager segmentDataManager = tableDataManager.acquireSegment(segmentName);
+    if (segmentDataManager == null) {
+      throw new WebApplicationException(
+              String.format("Table %s segment %s does not exist", realtimeTableName, segmentName),
+              Response.Status.NOT_FOUND);
+    }
+
+    File segmentTarFile = null;
+    try {
+      // Create the tar.gz segment file in the server's segmentTarUploadDir folder with a unique file name.
+      File segmentTarUploadDir =
+              new File(_serverInstance.getInstanceDataManager().getSegmentFileDirectory(), SEGMENT_UPLOAD_DIR);
+      segmentTarUploadDir.mkdir();
+
+      segmentTarFile = new File(segmentTarUploadDir, tableNameWithType + "_" + segmentName + "_" + UUID.randomUUID()
+              + TarGzCompressionUtils.TAR_GZ_FILE_EXTENSION);
+      TarGzCompressionUtils.createTarGzFile(new File(tableDataManager.getTableDataDir(), segmentName), segmentTarFile);
+
+      // Use segment uploader to upload the segment tar file to segment store and return the segment download url.
+      SegmentUploader segmentUploader = _serverInstance.getInstanceDataManager().getSegmentUploader();
+      URI segmentDownloadUrl = segmentUploader.uploadSegment(segmentTarFile, new LLCSegmentName(segmentName));
+      return segmentDownloadUrl == null ? Strings.EMPTY : segmentDownloadUrl.getPath();

Review comment:
       I would throw a failure if segmentDownloadUrl turns out to be null (this means upload failed).




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

[GitHub] [incubator-pinot] liuchang0520 commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r602908054



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/InstanceDataManager.java
##########
@@ -132,4 +133,9 @@ void reloadAllSegments(String tableNameWithType)
    * Returns the Helix property store.
    */
   ZkHelixPropertyStore<ZNRecord> getPropertyStore();
+
+  /**
+   * Returns the segment uploader.

Review comment:
       Comments updated.




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

[GitHub] [incubator-pinot] chenboat commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r602899718



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/InstanceDataManager.java
##########
@@ -132,4 +133,9 @@ void reloadAllSegments(String tableNameWithType)
    * Returns the Helix property store.
    */
   ZkHelixPropertyStore<ZNRecord> getPropertyStore();
+
+  /**
+   * Returns the segment uploader.

Review comment:
       Add more details to the javadoc: explain what a segment uploader does and how server can use it. 




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

[GitHub] [incubator-pinot] chenboat commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r593371836



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -50,6 +50,8 @@
   public static final String INSTANCE_SEGMENT_TAR_DIR = "segmentTarDir";
   // Key of segment directory
   public static final String INSTANCE_BOOTSTRAP_SEGMENT_DIR = "bootstrap.segment.dir";
+  // Key of segment store uri
+  public static final String SEGMENT_STORE_URI = "segment.store.uri";

Review comment:
       Currently in IndexLoadingConfig, there is also a field called "segment.store.uri" used by Pinot servers to upload segments to deepstore during LLC protocol. We do not want to set the deep store uri in two difference places obviously. I think it is good to put it here. Can you (1) remove the field in IndexLoadingConfig and (2) make all users to that field to use InstanceDataManagerConfig's "segment.store.uri" instead?




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

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791825604


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=h1) Report
   > Merging [#6653](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=desc) (6a73876) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `0.43%`.
   > The diff coverage is `62.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6653/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6653      +/-   ##
   ==========================================
   - Coverage   66.44%   66.01%   -0.44%     
   ==========================================
     Files        1075     1396     +321     
     Lines       54773    67797   +13024     
     Branches     8168     9811    +1643     
   ==========================================
   + Hits        36396    44758    +8362     
   - Misses      15700    19866    +4166     
   - Partials     2677     3173     +496     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `66.01% <62.64%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
   | [...pinot/broker/api/resources/PinotClientRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdENsaWVudFJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (-27.28%)` | :arrow_down: |
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <ø> (+9.52%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <ø> (-13.29%)` | :arrow_down: |
   | [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `82.85% <ø> (+10.12%)` | :arrow_up: |
   | [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
   | ... and [1282 more](https://codecov.io/gh/apache/incubator-pinot/pull/6653/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=footer). Last update [a84846e...6a73876](https://codecov.io/gh/apache/incubator-pinot/pull/6653?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

[GitHub] [incubator-pinot] liuchang0520 removed a comment on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 removed a comment on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791840974


   > Hi @liuchang0520, this feature is already being worked on at #6567.
   > 
   > Are you aware of 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

[GitHub] [incubator-pinot] chenboat merged pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
chenboat merged pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653


   


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

[GitHub] [incubator-pinot] liuchang0520 commented on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791844297


   > Hi @liuchang0520, this feature is already being worked on at #6567.
   > 
   > Are you aware of this?
   
   Hi @snleee , my understanding is this change covers a different segment upload path: server uploads LLC segment to segment store, though I am not sure. @chenboat Is there an overlap between this pr and #6567 ?


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

[GitHub] [incubator-pinot] liuchang0520 commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r598475935



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -50,6 +50,8 @@
   public static final String INSTANCE_SEGMENT_TAR_DIR = "segmentTarDir";
   // Key of segment directory
   public static final String INSTANCE_BOOTSTRAP_SEGMENT_DIR = "bootstrap.segment.dir";
+  // Key of segment store uri
+  public static final String SEGMENT_STORE_URI = "segment.store.uri";

Review comment:
       Since import HelixInstanceDataManagerConfig's "segment.store.uri" would cause circular dependency between pinot-server and pinot-core, I added this config key to CommonConstants.




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

[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r594564224



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url
+  @POST
+  @Path("/segments/{realtimeTableName}/{segmentName}")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Upload a low level consumer segment to segment store and return the segment download url", notes = "Upload a low level consumer segment to segment store and return the segment download url")
+  @ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error", response = ErrorInfo.class), @ApiResponse(code = 404, message = "Table or segment not found", response = ErrorInfo.class), @ApiResponse(code = 400, message = "Bad request", response = ErrorInfo.class)})
+  public String uploadLLCSegment(
+          @ApiParam(value = "Name of the REALTIME table", required = true) @PathParam("realtimeTableName") String realtimeTableName,
+          @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") String segmentName) throws Exception {
+    LOGGER.info("Received a request to upload low level consumer segment {} for table {}", segmentName, realtimeTableName);
+
+    // Check it's realtime table
+    TableType tableType = TableNameBuilder.getTableTypeFromTableName(realtimeTableName);
+    if (TableType.OFFLINE == tableType) {
+      throw new WebApplicationException(
+              String.format("Cannot upload low level consumer segment for OFFLINE table: %s", realtimeTableName),
+              Response.Status.BAD_REQUEST);
+    }
+
+    // Check the segment is low level consumer segment
+    if (!LLCSegmentName.isLowLevelConsumerSegmentName(segmentName)) {
+      throw new WebApplicationException(
+              String.format("Segment %s is not a low level consumer segment", segmentName),
+              Response.Status.BAD_REQUEST);
+    }
+
+    String tableNameWithType = TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(realtimeTableName);
+    TableDataManager tableDataManager = checkGetTableDataManager(tableNameWithType);
+    SegmentDataManager segmentDataManager = tableDataManager.acquireSegment(segmentName);
+    if (segmentDataManager == null) {
+      throw new WebApplicationException(
+              String.format("Table %s segment %s does not exist", realtimeTableName, segmentName),
+              Response.Status.NOT_FOUND);
+    }
+
+    File segmentTarFile = null;
+    try {
+      // Create the tar.gz segment file in the server's segmentTarUploadDir folder with a unique file name.
+      File segmentTarUploadDir =
+              new File(_serverInstance.getInstanceDataManager().getSegmentFileDirectory(), SEGMENT_UPLOAD_DIR);
+      segmentTarUploadDir.mkdir();
+
+      segmentTarFile = new File(segmentTarUploadDir, tableNameWithType + "_" + segmentName + "_" + UUID.randomUUID()
+              + TarGzCompressionUtils.TAR_GZ_FILE_EXTENSION);
+      TarGzCompressionUtils.createTarGzFile(new File(tableDataManager.getTableDataDir(), segmentName), segmentTarFile);
+
+      // Use segment uploader to upload the segment tar file to segment store and return the segment download url.
+      SegmentUploader segmentUploader = _serverInstance.getInstanceDataManager().getSegmentUploader();
+      URI segmentDownloadUrl = segmentUploader.uploadSegment(segmentTarFile, new LLCSegmentName(segmentName));
+      return segmentDownloadUrl == null ? Strings.EMPTY : segmentDownloadUrl.getPath();

Review comment:
       +1

##########
File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
##########
@@ -50,6 +50,8 @@
   public static final String INSTANCE_SEGMENT_TAR_DIR = "segmentTarDir";
   // Key of segment directory
   public static final String INSTANCE_BOOTSTRAP_SEGMENT_DIR = "bootstrap.segment.dir";
+  // Key of segment store uri
+  public static final String SEGMENT_STORE_URI = "segment.store.uri";

Review comment:
       +1

##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -24,36 +24,34 @@
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import java.io.File;
+import java.net.URI;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import javax.inject.Inject;
-import javax.ws.rs.DefaultValue;
-import javax.ws.rs.Encoded;
-import javax.ws.rs.GET;
-import javax.ws.rs.Path;
-import javax.ws.rs.PathParam;
-import javax.ws.rs.Produces;
-import javax.ws.rs.QueryParam;
-import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.*;

Review comment:
       (Code style) Please use the `Pinot Style`: https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#setup-ide

##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -24,36 +24,34 @@
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import java.io.File;
+import java.net.URI;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import javax.inject.Inject;
-import javax.ws.rs.DefaultValue;
-import javax.ws.rs.Encoded;
-import javax.ws.rs.GET;
-import javax.ws.rs.Path;
-import javax.ws.rs.PathParam;
-import javax.ws.rs.Produces;
-import javax.ws.rs.QueryParam;
-import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.*;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.StreamingOutput;
+
+import joptsimple.internal.Strings;

Review comment:
       Don't import this random library




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

[GitHub] [incubator-pinot] chenboat commented on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
chenboat commented on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-792985166


   Yes. This PR is about deep store by pass and not related to #6567. The added endpoint is for the Pinot servers to upload segments to deep store. 


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

[GitHub] [incubator-pinot] snleee commented on pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
snleee commented on pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#issuecomment-791833134


   Hi @liuchang0520, similar work is currently worked from https://github.com/apache/incubator-pinot/pull/6567. 
   
   Are you aware of 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

[GitHub] [incubator-pinot] liuchang0520 commented on a change in pull request #6653: add uploadLLCSegment endpoint in TableResource

Posted by GitBox <gi...@apache.org>.
liuchang0520 commented on a change in pull request #6653:
URL: https://github.com/apache/incubator-pinot/pull/6653#discussion_r598476092



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -244,6 +243,62 @@ public Response downloadSegment(
     }
   }
 
+  // Upload a low level consumer segment to segment store and return the segment download url
+  @POST
+  @Path("/segments/{realtimeTableName}/{segmentName}")

Review comment:
       Updated the endpoint path.




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