imply-cheddar commented on PR #13852: URL: https://github.com/apache/druid/pull/13852#issuecomment-1453044252
This change does not deserve a new config and we cannot merge it in a way that requires a new config. If we separate out to a queue that is read, then the Duty stays the same and populates that queue, the thread just runs stuff based on whatever is in that queue. It does not need an extra config and does not need any extra thought by end users. It will immediately benefit all deployments. Adding more configs is a very expensive operation and adding a new config that is hard to understand (why would we be setting the reset *longer* than something else?) is even worse, especially for functionality that will benefit all deployments and that we want on for everybody. Please fix the PR to remove all new configs and make it just operate by having the duty mutate a queue (each time it wakes up) and have a new thread that reads from that queue and submits jobs in accordance with whatever configurations are set in terms of how many task slots can be used. As it is, I'm -1 on this PR until these changes are made. -- 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]
