[GitHub] [incubator-pinot] mcvsubbu commented on issue #6966: Issues with recent changes on upload segment endpoint

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

mcvsubbu commented on issue #6966:
URL: https://github.com/apache/incubator-pinot/issues/6966#issuecomment-847457949


   > I believe the problem is on `PinotSegmentUploadDownloadRestletResource` line 295 where `tableNameWithType` is mistakenly passed to the `getZkDownloadURIForSegmentUpload()` where `rawTableName` is expected.
   > 
   > Since this PR has been merged for more than 1 months, should we consider changing the behavior to use `tableNameWithType` in the segment path, or revert it back to the old behavior of using `rawTableName`?
   > 
   > The advantage of using `tableNameWithType` in the segment path is to separate the segments for offline and realtime, and allow same segment name for offline and realtime table (might not be desired though). But this is changing the existing behavior, and might cause other classes to not work properly (such as `SegmentDeletionManager` mentioned above)
   
   Exactly. We should fix the controller to do the right thing -- drop the segment in the right place. Otherwise, we open up to other issues that may only surface much later, making production systems take a huge repair time. Right now, the only thing we could find is retention (the new segments uploaded will not get retained out).  But there is no saying what else is out there.


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