drcrallen commented on a change in pull request #6629: Add support parallel
combine in brokers
URL: https://github.com/apache/incubator-druid/pull/6629#discussion_r240791896
##########
File path:
server/src/main/java/org/apache/druid/client/selector/QueryableDruidServer.java
##########
@@ -19,38 +19,16 @@
package org.apache.druid.client.selector;
-import org.apache.druid.client.DirectDruidClient;
import org.apache.druid.client.DruidServer;
+import org.apache.druid.query.QueryRunner;
/**
+ * Interface to represent a single server which can participate in query
processing. The main implementation is
+ * {@link RemoteDruidServer}.
*/
-public class QueryableDruidServer
+public interface QueryableDruidServer<T extends QueryRunner>
{
- private final DruidServer server;
- private final DirectDruidClient client;
+ DruidServer getServer();
Review comment:
Is this really needed? The only confounding item I see here is that
`isSegmentReplicationTarget()` is called against `DruidServer` as a proxy for
"can I populate my cache off of this server's results", which also feels like a
weird way to determine cache policy.
The other uses for `DruidServer`'s getter are around metadata in segment
balancing at the coordinator. So maybe the `QueryableDruidServer` and
`DruidServer` are a bit overloaded in use cases. And it looks like priority and
metadata are available in `DruidServerMetadata`, so maybe it should simply be
`getQueryRunner()` and `getServerMetadata()` as the two methods for
`QueryableDruidServer`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
