Jackie-Jiang commented on code in PR #11978:
URL: https://github.com/apache/pinot/pull/11978#discussion_r1408529178
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerDebug.java:
##########
@@ -152,7 +189,40 @@ public Map<ServerInstance, List<String>>
getRoutingTableForQuery(
@ApiParam(value = "SQL query (table name should have type suffix)")
@QueryParam("query") String query,
@Context HttpHeaders httpHeaders) {
BrokerRequest brokerRequest =
CalciteSqlCompiler.compileToBrokerRequest(query);
+ checkAccessControl(brokerRequest, httpHeaders);
+ RoutingTable routingTable = _routingManager.getRoutingTable(brokerRequest,
getRequestId());
+ if (routingTable != null) {
+ return
removeOptionalSegments(routingTable.getServerInstanceToSegmentsMap());
+ } else {
+ throw new WebApplicationException("Cannot find routing for query: " +
query, Response.Status.NOT_FOUND);
+ }
+ }
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/debug/routingTableWithOptionalSegments/sql")
+ @ManualAuthorization
+ @ApiOperation(value = "Get the routing table for a query, including optional
segments")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Routing table"),
+ @ApiResponse(code = 404, message = "Routing not found"),
+ @ApiResponse(code = 500, message = "Internal server error")
+ })
+ public Map<ServerInstance, Pair<List<String>, List<String>>>
getRoutingTableForQueryWithOptionalSegments(
+ @ApiParam(value = "SQL query (table name should have type suffix)")
@QueryParam("query") String query,
+ @Context HttpHeaders httpHeaders) {
+ Map<ServerInstance, Pair<List<String>, List<String>>> result;
Review Comment:
(minor) Remove
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -610,7 +611,42 @@ public RoutingTable getRoutingTable(BrokerRequest
brokerRequest, long requestId)
return null;
}
InstanceSelector.SelectionResult selectionResult =
routingEntry.calculateRouting(brokerRequest, requestId);
- Map<String, String> segmentToInstanceMap =
selectionResult.getSegmentToInstanceMap();
+ return new RoutingTable(getServerInstanceToSegmentsMap(tableNameWithType,
selectionResult),
+ selectionResult.getUnavailableSegments(),
selectionResult.getNumPrunedSegments());
+ }
+
+ private Map<ServerInstance, Pair<List<String>, List<String>>>
getServerInstanceToSegmentsMap(String tableNameWithType,
+ InstanceSelector.SelectionResult selectionResult) {
+ Map<ServerInstance, Pair<List<String>, List<String>>> merged = new
HashMap<>();
+ for (Map.Entry<String, String> entry :
selectionResult.getSegmentToInstanceMap().entrySet()) {
+ ServerInstance serverInstance =
_enabledServerInstanceMap.get(entry.getValue());
+ if (serverInstance != null) {
+ Pair<List<String>, List<String>> pair =
+ merged.computeIfAbsent(serverInstance, k -> Pair.of(new
ArrayList<>(), new ArrayList<>()));
+ pair.getLeft().add(entry.getKey());
+ } else {
+ // Should not happen in normal case unless encountered unexpected
exception when updating routing entries
+ _brokerMetrics.addMeteredTableValue(tableNameWithType,
BrokerMeter.SERVER_MISSING_FOR_ROUTING, 1L);
+ }
+ }
+ for (Map.Entry<String, String> entry :
selectionResult.getOptionalSegmentToInstanceMap().entrySet()) {
+ ServerInstance serverInstance =
_enabledServerInstanceMap.get(entry.getValue());
+ if (serverInstance != null) {
+ Pair<List<String>, List<String>> pair = merged.get(serverInstance);
+ // Skip servers that don't have non-optional segments, so that servers
always get some non-optional segments
+ // to process, to be backward compatible.
+ // TODO: allow servers only with optional segments
+ if (pair != null) {
+ pair.getRight().add(entry.getKey());
+ }
+ }
+ // TODO: Report missing server metrics when we allow servers only with
optional segments.
+ }
+ return merged;
+ }
+
+ private Map<ServerInstance, List<String>>
getServerInstanceToSegmentsMap(String tableNameWithType,
Review Comment:
This can also be removed
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/QueryRouter.java:
##########
@@ -195,16 +198,23 @@ void markQueryDone(long requestId) {
_asyncQueryResponseMap.remove(requestId);
}
- private InstanceRequest getInstanceRequest(long requestId, BrokerRequest
brokerRequest, List<String> segments) {
+ private InstanceRequest getInstanceRequest(long requestId, BrokerRequest
brokerRequest,
+ Pair<List<String>, List<String>> segments) {
InstanceRequest instanceRequest = new InstanceRequest();
instanceRequest.setRequestId(requestId);
instanceRequest.setQuery(brokerRequest);
Map<String, String> queryOptions =
brokerRequest.getPinotQuery().getQueryOptions();
if (queryOptions != null) {
instanceRequest.setEnableTrace(Boolean.parseBoolean(queryOptions.get(CommonConstants.Broker.Request.TRACE)));
}
- instanceRequest.setSearchSegments(segments);
+ instanceRequest.setSearchSegments(segments.getLeft());
instanceRequest.setBrokerId(_brokerId);
+ if (CollectionUtils.isNotEmpty(segments.getRight())) {
+ // Don't set this field, i.e. leave it as null, if there is no optional
segment at all, to be more backward
+ // compatible, as there are places like in multi-staged query engine
where this field is not set today when
Review Comment:
```suggestion
// compatible, as there are places like in multi-stage query engine
where this field is not set today when
```
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -621,8 +657,22 @@ public RoutingTable getRoutingTable(BrokerRequest
brokerRequest, long requestId)
_brokerMetrics.addMeteredTableValue(tableNameWithType,
BrokerMeter.SERVER_MISSING_FOR_ROUTING, 1L);
}
}
- return new RoutingTable(serverInstanceToSegmentsMap,
selectionResult.getUnavailableSegments(),
- selectionResult.getNumPrunedSegments());
+ return serverInstanceToSegmentsMap;
+ }
+
+ private static Map<ServerInstance, Pair<List<String>, List<String>>> merge(
Review Comment:
Remove it?
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
