You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/08/02 07:06:08 UTC
[GitHub] [druid] kfaraz opened a new pull request #11530: Configure Broker to connect to only allowed Historical tiers
kfaraz opened a new pull request #11530:
URL: https://github.com/apache/druid/pull/11530
### Description
This PR adds a config for the `CustomTierSelectorStrategy` which can
be used to specify a list of allowed Historical Tiers that the Broker can
connect to.
This can be used to have dedicated Brokers for different data tiers.
### Changes
* Add `allowedTiers` in `CustomTierSelectorStrategyConfig`
* Add property `druid.broker.select.tier.custom.allowedTiers`
* Override server `pick()` methods in `CustomTierSelectorStrategy`
to only pick servers from allowed tiers
<hr>
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [x] added documentation for new or modified features or behaviors.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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
[GitHub] [druid] kfaraz closed pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
kfaraz closed pull request #11530:
URL: https://github.com/apache/druid/pull/11530
--
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
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11530:
URL: https://github.com/apache/druid/pull/11530#discussion_r680706056
##########
File path: docs/configuration/index.md
##########
@@ -1605,7 +1605,8 @@ These Broker configurations can be defined in the `broker/runtime.properties` fi
|--------|---------------|-----------|-------|
|`druid.broker.balancer.type`|`random`, `connectionCount`|Determines how the broker balances connections to Historical processes. `random` choose randomly, `connectionCount` picks the process with the fewest number of active connections to|`random`|
|`druid.broker.select.tier`|`highestPriority`, `lowestPriority`, `custom`|If segments are cross-replicated across tiers in a cluster, you can tell the broker to prefer to select segments in a tier with a certain priority.|`highestPriority`|
-|`druid.broker.select.tier.custom.priorities`|`An array of integer priorities.` E.g., `[-1, 0, 1, 2]`|Select servers in tiers with a custom priority list.|The config only has effect if `druid.broker.select.tier` is set to `custom`. If `druid.broker.select.tier` is set to `custom` but this config is not specified, the effect is the same as `druid.broker.select.tier` set to `highestPriority`. Any of the integers in this config can be ignored if there's no corresponding tiers with such priorities. Tiers with priorities explicitly specified in this config always have higher priority than those not and those not specified fall back to use `highestPriority` strategy among themselves.|
+|`druid.broker.select.tier.custom.priorities`|An array of integer priorities. E.g., `[-1, 0, 1, 2]`|Select servers in tiers with a custom priority list.|The config only has effect if `druid.broker.select.tier` is set to `custom`. If `druid.broker.select.tier` is set to `custom` but this config is not specified, the effect is the same as `druid.broker.select.tier` set to `highestPriority`. Any of the integers in this config can be ignored if there's no corresponding tiers with such priorities. Tiers with priorities explicitly specified in this config always have higher priority than those not and those not specified fall back to use `highestPriority` strategy among themselves.|
+|`druid.broker.select.tier.custom.tierNames`|A non-empty array of tier names, e.g., `["_default_tier","_hot_tier"]`|Specifies the tier names to which the Broker can connect. The Broker cannot connect to a Historical process belonging to a tier not listed in this config.|All tiers are allowed if no value is specified for this config or if an empty array is specified. This config has effect only if `druid.broker.select.tier` is set to `custom`.|
Review comment:
```suggestion
|`druid.broker.select.tier.custom.allowedTierNames`|A non-empty array of tier names, e.g., `["_default_tier","_hot_tier"]`|Specifies the tier names to which the Broker can connect. The Broker cannot connect to a Historical process belonging to a tier not listed in this config.|All tiers are allowed if no value is specified for this config or if an empty array is specified. This config has effect only if `druid.broker.select.tier` is set to `custom`.|`[]`|
```
##########
File path: server/src/main/java/org/apache/druid/client/selector/CustomTierSelectorStrategyConfig.java
##########
@@ -31,8 +31,16 @@
@JsonProperty
private List<Integer> priorities = new ArrayList<>();
+ @JsonProperty
+ private List<String> allowedTiers = new ArrayList<>();
Review comment:
this name should be same as what comes after `druid.broker.select.tier.custom.`. so if you are using `druid.broker.select.tier.custom.tierNames`, then this field should be named `tierNames` or you should have the annotation as `@JsonProperty("tierNames")`
Please add a test for serde if not added already.
--
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
[GitHub] [druid] kfaraz commented on pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
kfaraz commented on pull request #11530:
URL: https://github.com/apache/druid/pull/11530#issuecomment-891167847
Thanks a lot for the suggestion, @dylwylie . I will try out the config `druid.broker.segment.watchedTiers` and see if this meets all the use cases I had in mind.
--
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
[GitHub] [druid] kfaraz commented on a change in pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
kfaraz commented on a change in pull request #11530:
URL: https://github.com/apache/druid/pull/11530#discussion_r680715477
##########
File path: server/src/main/java/org/apache/druid/client/selector/CustomTierSelectorStrategyConfig.java
##########
@@ -31,8 +31,16 @@
@JsonProperty
private List<Integer> priorities = new ArrayList<>();
+ @JsonProperty
+ private List<String> allowedTiers = new ArrayList<>();
Review comment:
I had changed these names. Must have missed changing here. Thanks!
--
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
[GitHub] [druid] dylwylie commented on pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
dylwylie commented on pull request #11530:
URL: https://github.com/apache/druid/pull/11530#issuecomment-891066761
Is there an advantage to using this new config option over the existing config :: druid.broker.segment.watchedTiers ?
Apprciate that they're not like for like but is there a use case the option added here fulfils that his one doesn't?
--
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
[GitHub] [druid] dylwylie edited a comment on pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
dylwylie edited a comment on pull request #11530:
URL: https://github.com/apache/druid/pull/11530#issuecomment-891066761
Is there an advantage to using this new config option over the existing config :: druid.broker.segment.watchedTiers ?
Apprciate that they're not like for like but is there a use case that the option added here fulfils that the existing one doesn't?
--
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
[GitHub] [druid] kfaraz commented on a change in pull request #11530: Configure Broker to connect to only allowed Historical tiers
Posted by GitBox <gi...@apache.org>.
kfaraz commented on a change in pull request #11530:
URL: https://github.com/apache/druid/pull/11530#discussion_r680719489
##########
File path: docs/configuration/index.md
##########
@@ -1605,7 +1605,8 @@ These Broker configurations can be defined in the `broker/runtime.properties` fi
|--------|---------------|-----------|-------|
|`druid.broker.balancer.type`|`random`, `connectionCount`|Determines how the broker balances connections to Historical processes. `random` choose randomly, `connectionCount` picks the process with the fewest number of active connections to|`random`|
|`druid.broker.select.tier`|`highestPriority`, `lowestPriority`, `custom`|If segments are cross-replicated across tiers in a cluster, you can tell the broker to prefer to select segments in a tier with a certain priority.|`highestPriority`|
-|`druid.broker.select.tier.custom.priorities`|`An array of integer priorities.` E.g., `[-1, 0, 1, 2]`|Select servers in tiers with a custom priority list.|The config only has effect if `druid.broker.select.tier` is set to `custom`. If `druid.broker.select.tier` is set to `custom` but this config is not specified, the effect is the same as `druid.broker.select.tier` set to `highestPriority`. Any of the integers in this config can be ignored if there's no corresponding tiers with such priorities. Tiers with priorities explicitly specified in this config always have higher priority than those not and those not specified fall back to use `highestPriority` strategy among themselves.|
+|`druid.broker.select.tier.custom.priorities`|An array of integer priorities. E.g., `[-1, 0, 1, 2]`|Select servers in tiers with a custom priority list.|The config only has effect if `druid.broker.select.tier` is set to `custom`. If `druid.broker.select.tier` is set to `custom` but this config is not specified, the effect is the same as `druid.broker.select.tier` set to `highestPriority`. Any of the integers in this config can be ignored if there's no corresponding tiers with such priorities. Tiers with priorities explicitly specified in this config always have higher priority than those not and those not specified fall back to use `highestPriority` strategy among themselves.|
+|`druid.broker.select.tier.custom.tierNames`|A non-empty array of tier names, e.g., `["_default_tier","_hot_tier"]`|Specifies the tier names to which the Broker can connect. The Broker cannot connect to a Historical process belonging to a tier not listed in this config.|All tiers are allowed if no value is specified for this config or if an empty array is specified. This config has effect only if `druid.broker.select.tier` is set to `custom`.|
Review comment:
Fixed the name of the config. The last `[]` is not needed as there are already four columns.
--
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