suneet-s commented on PR #13852: URL: https://github.com/apache/druid/pull/13852#issuecomment-1450147069
Thanks for the reviews @imply-cheddar and @kfaraz > Another way you could achieve the intended outcome and, I think, have a bit more straightforward code (at least, you wouldn't need any new configs) would be to introduce a new thread. I described this approach in the alternatives considered section of the PR. It does clean up the code a little bit (primarily that `CompactionSegmentSearchPolicy#resetIfNeeded` doesn't need to return a pair so callers know when the iterator was reset). However, we still end up with 2 configs - one for the period at which the iterator should be reset and another for the period at which compact tasks should be scheduled. The other reason I did not go with this approach is to preserve compatibility for users currently using the CompactSegments duty as a custom duty. Trying to disable the old way of running auto-compaction and enable 2 new duties to run it would make things more error prone imo. > The CoordinatorSimulation is intended to allow you to create tests that simulate states of clusters. Thanks for this. I did not know about this class, I will add some tests here. > The other advantages of this approach are that users can benefit from it even if they don't specify a custom period for CompactSegments. I have purposefully implemented this change in a way where there should be no user visible change yet. I would like to be able to enable this on a few production clusters and test this in real world scenarios before Druid users are opted in to this by default. A future patch should change the defaults so users benefit from this even if they make no configuration changes. -- 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]
