deepthi912 commented on code in PR #15360:
URL: https://github.com/apache/pinot/pull/15360#discussion_r2012553849
##########
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:
Instead of having 2 separate methods, would it make sense to fetch responses
using IS server set instead of tag based for `needReload` as well
##########
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:
Instead of having 2 separate methods, would it make sense to fetch responses
using IS server set instead of tag based for `needReload` as well?
--
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
