suneet-s commented on a change in pull request #10732:
URL: https://github.com/apache/druid/pull/10732#discussion_r556068500
##########
File path:
server/src/main/java/org/apache/druid/server/metrics/DruidMonitorSchedulerConfig.java
##########
@@ -28,9 +29,17 @@
*/
public class DruidMonitorSchedulerConfig extends MonitorSchedulerConfig
{
+ @JsonProperty
+ private String schedulerClassName =
ClockDriftSafeMonitorScheduler.class.getName();
Review comment:
I've also thought about this a bunch and have changed my opinion on
whether or not we should change the scheduler to a new dependency by default a
few times.
While changing to use the CronScheduler might fix a bug, it isn't clear
whether any users have run into this in the field. I thought about documenting
why a user would want to change the scheduler to CronScheduler instead of the
older implementation, and I couldn't think of a good user facing reason to do
so. So if we set the default to the old implementation, I don't think anyone
would test it in production, so it would continue to live as dead code, and
we'll have the same dilemma in the next release or 2 when we ask whether or not
this has been run in production.
Setting the default to the older implementation reduces the impact of any
bug that might show up in long running tests (even though this library was
specifically built to fix issues found with long running processes). The
drawback here is finding a reason for some users to try this in production so
that we can sunset the feature flag in a release or 2.
Writing out this comment, I now think the more cautious approach - keeping
the default the same - is better as it's hard to articulate the benefit for
switching the scheduling and taking on the risk associated with changing the
older behavior.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
