FrankChen021 commented on pull request #10363:
URL: https://github.com/apache/druid/pull/10363#issuecomment-689487649
Hi @jihoonson @suneet-s , here's a clarification for the change of user
facing configuration.
This PR has no code change with the configuration name, but a rectification
of the doc from the wrong configuration item name of selector strategy named as
`druid.segmentCache.locationSelectorStrategy` to
`druid.segmentCache.locationSelectorStrategy.type`.
When I first checked the issue, the doc also made me confused and took me
sometime on the question which was the right name.
Looking at the annotation of `StorageLocationSelectorStrategy`, it's bounded
to a property named as `type` to determine which class should be used during
deserialization.
```
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl =
LeastBytesUsedStorageLocationSelectorStrategy.class)
@JsonSubTypes(value = {
```
Based upon the code, the right configuration name should
`druid.segmentCache.locationSelectorStrategy.type`. The old implementation
always fails to deserialize strategy object in default configuration mode but
never throws any NPE exception because its getter method tries to initialize a
default strategy object when it finds the strategy object is null.
I don't know why the `druid.segmentCache.locationSelectorStrategy` was put
into the doc. Taking another class `BalancerStrategyFactory` for example, its
doc corresponding to its property name in code.
```
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "strategy", defaultImpl
= CostBalancerStrategyFactory.class)
@JsonSubTypes(value = {
@JsonSubTypes.Type(name = "diskNormalized", value =
DiskNormalizedCostBalancerStrategyFactory.class),
@JsonSubTypes.Type(name = "cost", value =
CostBalancerStrategyFactory.class),
@JsonSubTypes.Type(name = "cachingCost", value =
CachingCostBalancerStrategyFactory.class),
@JsonSubTypes.Type(name = "random", value =
RandomBalancerStrategyFactory.class),
})
public interface BalancerStrategyFactory
{
```
```
JsonConfigProvider.bind(binder, "druid.coordinator.balancer",
BalancerStrategyFactory.class);
```
And the
[doc](https://github.com/apache/druid/blob/master/docs/configuration/index.md#coordinator-operation)
says the property name should be `druid.coordinator.balancer.strategy` instead
of `druid.coordinator.balancer`
----------------------------------------------------------------
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]
