snleee commented on a change in pull request #4464: Delete extra refreshed 
segments after segment push
URL: https://github.com/apache/incubator-pinot/pull/4464#discussion_r306987360
 
 

 ##########
 File path: 
pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentTarPushJob.java
 ##########
 @@ -48,10 +54,45 @@ public void run()
       throws Exception {
     FileSystem fileSystem = FileSystem.get(_conf);
     try (ControllerRestApi controllerRestApi = getControllerRestApi()) {
-      controllerRestApi.pushSegments(fileSystem, 
getDataFilePaths(_segmentPattern));
+      // TODO: Deal with invalid prefixes in the future
 
 Review comment:
   I'm not sure if segment name based solution is the correct approach for 
deleting segments pushed together. The logic will break for custom segment name 
builder. Also, it will break in the case where people are pushing to the table 
from multiple push jobs.
   
   One approach that I can think of is to mark some id (e.g. batchId) to 
segments from offline push job and delete segments with the same batchId. 

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to