somandal commented on code in PR #15360:
URL: https://github.com/apache/pinot/pull/15360#discussion_r2012571938
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java:
##########
@@ -61,34 +61,58 @@ public TableMetadataReader(Executor executor,
HttpClientConnectionManager connec
/**
* Check if segments need a reload on any servers
- * @return pair of: a) number of failed responses, b) reload responses
returned
+ * @return response containing a) number of failed responses, b) reload
responses returned
*/
public TableReloadJsonResponse getServerCheckSegmentsReloadMetadata(String
tableNameWithType,
int timeoutMs)
throws InvalidConfigException, IOException {
- ServerSegmentMetadataReader.TableReloadResponse segmentsMetadataPair =
- getReloadCheckResponses(tableNameWithType, timeoutMs);
- List<String> segmentsMetadata =
segmentsMetadataPair.getServerReloadResponses();
- Map<String, JsonNode> response = new HashMap<>();
- for (String segmentMetadata : segmentsMetadata) {
- JsonNode responseJson = JsonUtils.stringToJsonNode(segmentMetadata);
- response.put(responseJson.get("instanceId").asText(), responseJson);
- }
- return new
TableReloadJsonResponse(segmentsMetadataPair.getNumFailedResponses(), response);
+ ServerSegmentMetadataReader.TableReloadResponse segmentsMetadataResponse =
getReloadCheckResponses(
+ tableNameWithType, timeoutMs);
+ return processSegmentMetadataReloadResponse(segmentsMetadataResponse);
}
public ServerSegmentMetadataReader.TableReloadResponse
getReloadCheckResponses(String tableNameWithType,
int timeoutMs) throws InvalidConfigException {
TableType tableType =
TableNameBuilder.getTableTypeFromTableName(tableNameWithType);
List<String> serverInstances =
_pinotHelixResourceManager.getServerInstancesForTable(tableNameWithType,
tableType);
Set<String> serverInstanceSet = new HashSet<>(serverInstances);
+ return getServerSetReloadCheckResponses(tableNameWithType, timeoutMs,
serverInstanceSet);
+ }
+
+ /**
+ * Check if segments need a reload on any servers based on a provided server
list (useful for rebalance where the
+ * server list may not match the currently tagged server list)
+ * @return response containing a) number of failed responses, b) reload
responses returned
+ */
+ public TableReloadJsonResponse
getServerSetCheckSegmentsReloadMetadata(String tableNameWithType,
Review Comment:
@deepthi912 and I discussed this and decided it is okay to keep them
separate.
Rebalance is the only scenario where the instance tags can differ from the
server list in the IdealState, and this pre-check needs to check this
in-between rebalances state. Most of the time the current mechanism to fetch
servers based on tags is correct. If servers are tagged / untagged, we do need
to run a rebalance for changes to take effect.
--
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
