[GitHub] [druid] danprince1 commented on a change in pull request #12013: Create a standalone primary replicant loader in the Coordinator

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

danprince1 commented on a change in pull request #12013:
URL: https://github.com/apache/druid/pull/12013#discussion_r776803870



##########
File path: docs/configuration/index.md
##########
@@ -781,7 +781,10 @@ These Coordinator static configurations can be defined in the `coordinator/runti
 
 |Property|Description|Default|
 |--------|-----------|-------|
+|`druid.coordinator.dutiesRunnableExecutor.threadPoolSize`|The number of threads used by the `ScheduledExecutorService` that executes `DutiesRunnable` objects|1|
 |`druid.coordinator.period`|The run period for the Coordinator. The Coordinator operates by maintaining the current state of the world in memory and periodically looking at the set of "used" segments and segments being served to make decisions about whether any changes need to be made to the data topology. This property sets the delay between each of these runs.|PT60S|
+|`druid.coordinator.loadPrimaryReplicantSeparately`|Flag that indicates if the Coordinator should put primary replicant loading for used segments on its own event loop. This will require more memory and CPU to be used by the coordinator, but will ensure that the loading of primary replicas for used segments is not blocked by other coordinator jobs. The default of false is likely fine for most clusters. Enabling it could make sense if you regularly have to wait longer than you'd like for coordination cycles to complete before primary replicants are loaded for used segments that are not being served by the cluster.|false|

Review comment:
       The idea of validating the config is a good one, and I went down a rabbit hole investigating how configurations are validated elsewhere in the druid code.  There are two schemes in use in the codebase for getting config properties deserialized into java objects.  The first uses the [skife](https://github.com/brianm/config-magic) library, and the second is a home-grown class that integrates validation annotations (see `JsonConfigurator`).  Apparently there is a (very slow) movement under way to standardize on the second method (see #920).  Unfortunately `DruidCoordinatorConfig` still uses the old way, which doesn't provide any validation.  So, the 'right' way to fix this would be to migrate coordinator config to the new way and add validation.  This would also require a custom validator, as none of the existing ones cover the relationship between two different properties.
   
   For now I'll add the doc and a simple check in DruidCoordinator.




-- 
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: commits-unsubscribe@druid.apache.org

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