zachjsh commented on PR #14662:
URL: https://github.com/apache/druid/pull/14662#issuecomment-1660749208

   @suneet-s 
   
   > This part of the patch seems substantial enough to be it's own PR, and 
should definitely be called out in the release notes, but as written, it is 
very easy to miss this. wrt killTaskSlotRatio, have you considered combining 
this with the compactTaskSlotRatio so that an operator has one knob to tune how 
much capacity in the cluster should be given to "data management" operations? 
Compaction tasks also offer a maxCompactTaskSlot config, if we do decide to 
have a different set of knobs for kill tasks - is there a reason not to also 
introduce a maxKillTaskSlots config?
   
   I thought was better to keep separate. I didnt want to allow for starvation 
of either auto kill or  auto compaction tasks because of the other.
   
   > Since we are now introducing a config to limit the number of concurrent 
kill tasks - is there any reason the kill duty should not run more frequently, 
say every minute by default?
   
   I believe that the period must be at least greater than the indexer period, 
which I believe is defaulted to 30 minutes. There is a validation check for 
this at the moment. I believe the coorinator kill duty is spawned from the 
indexer loop which runs at the 30 minutes I noted.
   
   > Are there any guardrails preventing an operator from setting the task 
slots used for data management operations higher than the available capacity of 
the cluster?
   
   The ratio config is used to calculate the task slot usage allotted from the 
total task slots in cluster (including autoscaling if configured). So if ratio 
is set to 0.10, the user can at any moment use 10% of total task slots on auto 
kill tasks.
   
   > WRT the kill_by_segment_id task - Why does this task need a different name 
from the kill task? Have you considered making the kill task accept an interval 
and a limit instead of a list of segment ids? This seems like an easier 
interface for operators to use and reason about.
   
   I actually had this exact implementation initially, but based on comments 
made in this review have changed approach. Let me know what you think


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to