pinot icon indicating copy to clipboard operation
pinot copied to clipboard
verified publisher icon apache

Add API to update ZK metadata of a segment

Open harold-kfuse opened this issue 2 years ago • 10 comments

Looking at BaseSingleSegmentConversionExecutor, I see that it always uploads the segment, even if the segment did not change but only the metadata (e.g., updating custom map for some minion task ran on the segment). The controller then checks that the segment did not change and only updates ZK.

The network transfer of the segment from minion task to controller seems wasteful in this case, since the minion task should already know that the segment did not change. If there is an API to simply update ZK metadata, then that will be more efficient.

harold-kfuse avatar Mar 22 '23 20:03 harold-kfuse

Hi, can I pick this up?

Vaas-Arora avatar Mar 28 '23 05:03 Vaas-Arora

@harold-kfuse Can you point me to the controller where the unchanged segment check is made? I am unable to find it.

maahir22 avatar Mar 30 '23 06:03 maahir22

@maahir22

https://github.com/apache/pinot/blob/d97f2d92bc6cdb1f6bafb7fd98f69afae1266caa/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java#L198

harold-kfuse avatar Mar 30 '23 18:03 harold-kfuse

@harold-kfuse, based on my analysis I see that

  1. PinotSegmentUploadDownloadRestletResource already has code to handle segment METADATA, I assume the same endpoint can be used for BaseSingleSegmentConversionExecutor also
  2. BaseMultipleSegmentsConversionExecutor handles segment METADATA upload, the same can be extended in BaseSingleSegmentConversionExecutor for METADATA upload

Please feel free to correct me if wrong.

pushp-ranja avatar Oct 17 '23 18:10 pushp-ranja

@heatclub Ideally we don't need to upload anything when the segment remains exactly the same. I guess the proposal is to add an API to only modify some custom fields in the segment ZK metadata

Jackie-Jiang avatar Oct 29 '23 16:10 Jackie-Jiang

Thanks @Jackie-Jiang , I was able to go through the code and get that part. Let me share the PR once its ready.

pushp-ranja avatar Oct 30 '23 01:10 pushp-ranja

Hi @Jackie-Jiang, I see that there hasn't been any update on this ticket for a while. Can I look into this if @heatclub isn't working on it?

ceekay47 avatar Apr 05 '24 07:04 ceekay47

Hey @ceekay47 please feel free to pick this up

pushp-ranja avatar Apr 07 '24 18:04 pushp-ranja

Hey, @ceekay47 can I look into this if you aren't working on it? Also, @Jackie-Jiang is this issue still needed?

deadb0d4 avatar Oct 06 '24 21:10 deadb0d4

Yes, the issue still applies. We do have a workaround by calling the ZK write rest API, but having a dedicate API and perform some validation is preferred

Jackie-Jiang avatar Oct 08 '24 21:10 Jackie-Jiang