manakbisht commented on code in PR #20222:
URL: https://github.com/apache/pinot/pull/20222#discussion_r4537432304
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java:
##########
@@ -131,92 +131,38 @@ private TableReloadJsonResponse
processSegmentMetadataReloadResponse(
public JsonNode getSegmentsMetadata(String tableNameWithType, @Nullable
List<String> columns,
@Nullable List<String> segments, int timeoutMs)
throws InvalidConfigException, IOException {
- return getSegmentsMetadataInternal(tableNameWithType, columns, segments,
timeoutMs);
- }
+ Map<String, List<String>> serverToSegs =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ BiMap<String, String> endpoints =
+
_pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegs.keySet());
+ ServerSegmentMetadataReader reader =
+ new ServerSegmentMetadataReader(_executor, _connectionManager,
+ _pinotHelixResourceManager.getServerAdminAuthProvider());
- /// Common helper used by both the new (server-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 {
- CompletionServiceHelper cs = new CompletionServiceHelper(_executor,
_connectionManager, endpoints,
+ List<String> tableUrls = new ArrayList<>(serverToSegs.size());
+ for (String server : serverToSegs.keySet()) {
+ tableUrls.add(reader.generateTableMetadataServerURL(
+ tableNameWithType, columns, segments, endpoints.get(server)));
+ }
+
+ CompletionServiceHelper cs = new CompletionServiceHelper(_executor,
_connectionManager, endpoints.inverse(),
_pinotHelixResourceManager.getServerAdminAuthProvider());
CompletionServiceHelper.CompletionServiceResponse resp =
- cs.doMultiGetRequest(urls, tableNameWithType, perSegmentJson,
timeoutMs);
- // all requests will fail if new server endpoint is not available
+ cs.doMultiGetRequest(tableUrls, tableNameWithType, false, timeoutMs);
if (resp._failedResponseCount > 0) {
Review Comment:
Agreed that the fallback also ran on current servers, but it didn't help
there. With the default 30s timeout, a current server only fails or times out
on the table-level request when the response is large enough to overload it,
e.g. columns=* on a large table. That request keeps running on the server after
the controller times out. The fallback then repeated the same work as one
request per segment, on top of the original, so it was likely to fail too.
Failing fast is the intended behavior.
--
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]
