Jackie-Jiang commented on code in PR #14250:
URL: https://github.com/apache/pinot/pull/14250#discussion_r2150680856
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java:
##########
@@ -127,50 +130,111 @@ private TableReloadJsonResponse
processSegmentMetadataReloadResponse(
/**
* This api takes in list of segments for which we need the metadata.
+ * This calls the server to get the metadata for all segments instead of
making a call per segment.
*/
public JsonNode getSegmentsMetadata(String tableNameWithType, List<String>
columns, Set<String> segmentsToInclude,
int timeoutMs)
throws InvalidConfigException, IOException {
return getSegmentsMetadataInternal(tableNameWithType, columns,
segmentsToInclude, timeoutMs);
}
- private JsonNode getSegmentsMetadataInternal(String tableNameWithType,
List<String> columns,
- Set<String> segmentsToInclude, int timeoutMs)
+ /**
+ * Common helper used by both the new (table-level) and legacy
+ * (segment-level) endpoints.
+ */
+ private JsonNode fetchAndAggregateMetadata(List<String> urls,
+ BiMap<String, String> endpoints,
+ boolean perSegmentJson,
+ String tableNameWithType,
+ int timeoutMs)
throws InvalidConfigException, IOException {
- final Map<String, List<String>> serverToSegmentsMap =
- _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
- BiMap<String, String> endpoints =
-
_pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
- ServerSegmentMetadataReader serverSegmentMetadataReader =
- new ServerSegmentMetadataReader(_executor, _connectionManager);
- // Filter segments that we need
- for (Map.Entry<String, List<String>> serverToSegment :
serverToSegmentsMap.entrySet()) {
- List<String> segments = serverToSegment.getValue();
- if (segmentsToInclude != null && !segmentsToInclude.isEmpty()) {
- segments.retainAll(segmentsToInclude);
+ CompletionServiceHelper cs =
+ new CompletionServiceHelper(_executor, _connectionManager, endpoints);
+ CompletionServiceHelper.CompletionServiceResponse resp =
+ cs.doMultiGetRequest(urls, tableNameWithType, perSegmentJson,
timeoutMs);
+
+ if (resp._failedResponseCount == urls.size()) {
+ throw new InvalidConfigException("All requests to server instances
failed.");
+ }
+
+ ObjectMapper mapper = new ObjectMapper();
+ ObjectNode aggregatedNode = mapper.createObjectNode();
+
+ for (String body : resp._httpResponses.values()) {
+ JsonNode node = JsonUtils.stringToJsonNode(body);
+
+ // legacy returns one JSON per segment; new returns one JSON with many
fields
+ if (perSegmentJson) {
+ String segmentName = node.get("segmentName").asText();
+ aggregatedNode.set(segmentName, node);
+ } else {
+ node.fields().forEachRemaining(entry ->
aggregatedNode.set(entry.getKey(), entry.getValue()));
}
}
+ return aggregatedNode;
+ }
- List<String> segmentsMetadata =
-
serverSegmentMetadataReader.getSegmentMetadataFromServer(tableNameWithType,
serverToSegmentsMap, endpoints,
- columns, timeoutMs);
- Map<String, JsonNode> response = new HashMap<>();
- for (String segmentMetadata : segmentsMetadata) {
- JsonNode responseJson = JsonUtils.stringToJsonNode(segmentMetadata);
- response.put(responseJson.get("segmentName").asText(), responseJson);
+ private List<String> buildTableLevelUrls(Map<String, List<String>>
serverToSegs,
+ BiMap<String, String> endpoints,
+ String tableNameWithType,
+ List<String> columns,
+ Set<String> segmentsFilter,
+ ServerSegmentMetadataReader reader) {
+
+ List<String> urls = new ArrayList<>(serverToSegs.size());
+ for (String server : serverToSegs.keySet()) {
+ urls.add(reader.generateTableMetadataServerURL(
+ tableNameWithType, columns, segmentsFilter, endpoints.get(server)));
}
- return JsonUtils.objectToJsonNode(response);
+ return urls;
}
- /**
- * This method retrieves the full segment metadata for a given table.
- * Currently supports only OFFLINE tables.
- * @return a map of segmentName to its metadata
- */
- public JsonNode getSegmentsMetadata(String tableNameWithType, List<String>
columns, int timeoutMs)
+ private List<String> buildSegmentLevelUrls(Map<String, List<String>>
serverToSegs,
+ BiMap<String, String> endpoints,
+ String tableNameWithType,
+ List<String> columns,
+ Set<String> segmentsFilter,
+ ServerSegmentMetadataReader reader) {
+
+ List<String> urls = new ArrayList<>();
+ for (Map.Entry<String, List<String>> e : serverToSegs.entrySet()) {
+ for (String segment : e.getValue()) {
+ if (segmentsFilter == null || segmentsFilter.isEmpty()
+ || segmentsFilter.contains(segment)) {
+ urls.add(reader.generateSegmentMetadataServerURL(
+ tableNameWithType, segment, columns, endpoints.get(e.getKey())));
+ }
+ }
+ }
+ return urls;
+ }
+
+ private JsonNode getSegmentsMetadataInternal(String tableNameWithType,
List<String> columns,
+ Set<String> segmentsToInclude, int timeoutMs)
throws InvalidConfigException, IOException {
- return getSegmentsMetadataInternal(tableNameWithType, columns, null,
timeoutMs);
+ Map<String, List<String>> serverToSegs =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ BiMap<String, String> endpoints =
+
_pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegs.keySet());
+ ServerSegmentMetadataReader reader =
+ new ServerSegmentMetadataReader(_executor, _connectionManager);
+
+ // try table level endpoint first
+ try {
+ List<String> tableUrls = buildTableLevelUrls(serverToSegs, endpoints,
+ tableNameWithType, columns, segmentsToInclude, reader);
+ return fetchAndAggregateMetadata(tableUrls, endpoints,
/*perSegmentJson=*/false,
+ tableNameWithType, timeoutMs);
+ } catch (InvalidConfigException ignore) {
Review Comment:
`InvalidConfigException` is thrown only when it fails to read the server
admin endpoint. If server doesn't support this API, I believe it can through a
different exception, and break the API. Can you try removing the server side
change and see if this method can correctly fall back to the old API?
--
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
