kfaraz commented on code in PR #19537:
URL: https://github.com/apache/druid/pull/19537#discussion_r3680574017
##########
server/src/main/java/org/apache/druid/client/selector/ServerSelector.java:
##########
@@ -190,16 +192,123 @@ public List<DruidServerMetadata>
getAllServers(CloneQueryMode cloneQueryMode)
}
@Nullable
- public <T> QueryableDruidServer pick(@Nullable Query<T> query,
CloneQueryMode cloneQueryMode)
+ public <T> QueryableDruidServer pick(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return pick(query, cloneQueryMode, getQueryableHistoricalTiers(query));
+ }
+
+ @Nullable
+ public <T> QueryableDruidServer pick(
+ @Nullable final Query<T> query,
+ final CloneQueryMode cloneQueryMode,
+ @Nullable final Set<String> queryableHistoricalTiers
+ )
{
synchronized (this) {
- if (!historicalServers.isEmpty()) {
- return historicalTierStrategy.pick(query,
filter.getQueryableServers(historicalServers, cloneQueryMode), segment.get());
+ if (queryableHistoricalTiers != null &&
queryableHistoricalTiers.isEmpty()) {
+ return null;
+ }
+ if (!historicalServers.isEmpty() || queryableHistoricalTiers != null) {
+ final Int2ObjectRBTreeMap<Set<QueryableDruidServer>>
queryableHistoricalServers =
+ getQueryableHistoricalServers(
+ filter.getQueryableServers(historicalServers, cloneQueryMode),
+ queryableHistoricalTiers
+ );
+ return historicalTierStrategy.pick(query, queryableHistoricalServers,
segment.get());
}
return realtimeTierStrategy.pick(query, realtimeServers, segment.get());
}
}
+ public <T> boolean hasQueryableServer(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return hasQueryableServer(getQueryableHistoricalTiers(query),
cloneQueryMode);
+ }
+
+ public boolean hasQueryableServer(
Review Comment:
Please add a short javadoc.
##########
server/src/main/java/org/apache/druid/client/selector/ServerSelector.java:
##########
@@ -190,16 +192,123 @@ public List<DruidServerMetadata>
getAllServers(CloneQueryMode cloneQueryMode)
}
@Nullable
- public <T> QueryableDruidServer pick(@Nullable Query<T> query,
CloneQueryMode cloneQueryMode)
+ public <T> QueryableDruidServer pick(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return pick(query, cloneQueryMode, getQueryableHistoricalTiers(query));
+ }
+
+ @Nullable
+ public <T> QueryableDruidServer pick(
+ @Nullable final Query<T> query,
+ final CloneQueryMode cloneQueryMode,
+ @Nullable final Set<String> queryableHistoricalTiers
+ )
{
synchronized (this) {
- if (!historicalServers.isEmpty()) {
- return historicalTierStrategy.pick(query,
filter.getQueryableServers(historicalServers, cloneQueryMode), segment.get());
+ if (queryableHistoricalTiers != null &&
queryableHistoricalTiers.isEmpty()) {
Review Comment:
This check can be done before the `synchronized` block.
##########
server/src/main/java/org/apache/druid/client/selector/ServerSelector.java:
##########
@@ -190,16 +192,123 @@ public List<DruidServerMetadata>
getAllServers(CloneQueryMode cloneQueryMode)
}
@Nullable
- public <T> QueryableDruidServer pick(@Nullable Query<T> query,
CloneQueryMode cloneQueryMode)
+ public <T> QueryableDruidServer pick(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return pick(query, cloneQueryMode, getQueryableHistoricalTiers(query));
+ }
+
+ @Nullable
+ public <T> QueryableDruidServer pick(
+ @Nullable final Query<T> query,
+ final CloneQueryMode cloneQueryMode,
+ @Nullable final Set<String> queryableHistoricalTiers
+ )
{
synchronized (this) {
- if (!historicalServers.isEmpty()) {
- return historicalTierStrategy.pick(query,
filter.getQueryableServers(historicalServers, cloneQueryMode), segment.get());
+ if (queryableHistoricalTiers != null &&
queryableHistoricalTiers.isEmpty()) {
+ return null;
+ }
+ if (!historicalServers.isEmpty() || queryableHistoricalTiers != null) {
+ final Int2ObjectRBTreeMap<Set<QueryableDruidServer>>
queryableHistoricalServers =
+ getQueryableHistoricalServers(
+ filter.getQueryableServers(historicalServers, cloneQueryMode),
+ queryableHistoricalTiers
Review Comment:
It would probably make more sense to pass the list of queryable tiers into
the `filter.getQueryableServers` since the function seems very closely related.
This would allow us to keep the server filtration logic in a single place,
and will also provide an easy way for tier alias expansion in
`BrokerViewOfCoordinatorConfig`, which is an impl of the `HistoricalFilter`.
##########
docs/querying/query-context-reference.md:
##########
@@ -68,9 +68,10 @@ Unless otherwise noted, the following parameters apply to
all query types, and t
|`useFilterCNF`|`false`| If true, Druid will attempt to convert the query
filter to Conjunctive Normal Form (CNF). During query processing, columns can
be pre-filtered by intersecting the bitmap indexes of all values that match the
eligible filters, often greatly reducing the raw number of rows which need to
be scanned. But this effect only happens for the top level filter, or
individual clauses of a top level 'and' filter. As such, filters in CNF
potentially have a higher chance to utilize a large amount of bitmap indexes on
string columns during pre-filtering. However, this setting should be used with
great caution, as it can sometimes have a negative effect on performance, and
in some cases, the act of computing CNF of a filter can be expensive. We
recommend hand tuning your filters to produce an optimal form if possible, or
at least verifying through experimentation that using this parameter actually
improves your query performance with no ill-effects.|
|`secondaryPartitionPruning`|`true`|Enable secondary partition pruning on the
Broker. The Broker will always prune unnecessary segments from the input scan
based on a filter on time intervals, but if the data is further partitioned
with hash or range partitioning, this option will enable additional pruning
based on a filter on secondary partition dimensions.|
|`debug`| `false` | Flag indicating whether to enable debugging outputs for
the query. When set to false, no additional logs will be produced (logs
produced will be entirely dependent on your logging level). When set to true,
the following addition logs will be produced:<br />- Log the stack trace of the
exception (if any) produced by the query |
-|`setProcessingThreadNames`|`false`| Flag indicating whether processing thread
names will be set to `processing_<queryId>` while processing a query. Thread
renaming aids in interpreting thread dumps, but has measurable thread renaming
overhead when segment scans are very quick. |
+|`setProcessingThreadNames`|`true`| Whether processing thread names will be
set to `queryType_dataSource_intervals` while processing a query. This aids in
interpreting thread dumps, and is on by default. Query overhead can be reduced
slightly by setting this to `false`. This has a tiny effect in most scenarios,
but can be meaningful in high-QPS, low-per-segment-processing-time scenarios. |
|`sqlPlannerBloat`|`1000`|Calcite parameter which controls whether to merge
two Project operators when inlining expressions causes complexity to increase.
Implemented as a workaround to exception `There are not enough rules to produce
a node with desired properties: convention=DRUID, sort=[]` thrown after
rejecting the merge of two projects.|
|`cloneQueryMode`|`excludeClones`| Indicates whether clone Historicals should
be queried by brokers. Clone servers are created by the `cloneServers`
Coordinator dynamic configuration. Possible values are `excludeClones`,
`includeClones` and `preferClones`. `excludeClones` means that clone
Historicals are not queried by the broker. `preferClones` indicates that when
given a choice between the clone Historical and the original Historical which
is being cloned, the broker chooses the clones. Historicals which are not
involved in the cloning process will still be queried. `includeClones` means
that broker queries any Historical without regarding clone status. This
parameter only affects native queries. MSQ does not query Historicals directly.|
+|`queryableHistoricalTiers`|`null`|Set of Historical tier names that may be
queried. When set, the Broker only queries Historical servers whose
`druid.server.tier` is in this set. Segments without a replica on one of the
listed tiers are skipped.|
Review Comment:
Does this also need to mention that tier aliases may be used instead of
actual tier names?
##########
server/src/main/java/org/apache/druid/client/selector/ServerSelector.java:
##########
@@ -190,16 +192,123 @@ public List<DruidServerMetadata>
getAllServers(CloneQueryMode cloneQueryMode)
}
@Nullable
- public <T> QueryableDruidServer pick(@Nullable Query<T> query,
CloneQueryMode cloneQueryMode)
+ public <T> QueryableDruidServer pick(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return pick(query, cloneQueryMode, getQueryableHistoricalTiers(query));
+ }
+
+ @Nullable
+ public <T> QueryableDruidServer pick(
+ @Nullable final Query<T> query,
+ final CloneQueryMode cloneQueryMode,
+ @Nullable final Set<String> queryableHistoricalTiers
+ )
{
synchronized (this) {
- if (!historicalServers.isEmpty()) {
- return historicalTierStrategy.pick(query,
filter.getQueryableServers(historicalServers, cloneQueryMode), segment.get());
+ if (queryableHistoricalTiers != null &&
queryableHistoricalTiers.isEmpty()) {
+ return null;
+ }
+ if (!historicalServers.isEmpty() || queryableHistoricalTiers != null) {
+ final Int2ObjectRBTreeMap<Set<QueryableDruidServer>>
queryableHistoricalServers =
+ getQueryableHistoricalServers(
+ filter.getQueryableServers(historicalServers, cloneQueryMode),
+ queryableHistoricalTiers
+ );
+ return historicalTierStrategy.pick(query, queryableHistoricalServers,
segment.get());
}
return realtimeTierStrategy.pick(query, realtimeServers, segment.get());
}
}
+ public <T> boolean hasQueryableServer(@Nullable final Query<T> query, final
CloneQueryMode cloneQueryMode)
+ {
+ return hasQueryableServer(getQueryableHistoricalTiers(query),
cloneQueryMode);
+ }
+
+ public boolean hasQueryableServer(
+ @Nullable final Set<String> queryableHistoricalTiers,
+ final CloneQueryMode cloneQueryMode
+ )
+ {
+ synchronized (this) {
+ if (queryableHistoricalTiers != null) {
Review Comment:
This check can happen outside the `synchronized` block.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
