[GitHub] [druid] suneet-s commented on a change in pull request #10732: Add a config for monitorScheduler type

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org