klsince commented on code in PR #12933:
URL: https://github.com/apache/pinot/pull/12933#discussion_r1566387588
##########
pinot-common/src/main/java/org/apache/pinot/core/util/PeerServerSegmentFinder.java:
##########
@@ -47,93 +46,73 @@ public class PeerServerSegmentFinder {
private PeerServerSegmentFinder() {
}
- private static final Logger _logger =
LoggerFactory.getLogger(PeerServerSegmentFinder.class);
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PeerServerSegmentFinder.class);
private static final int MAX_NUM_ATTEMPTS = 5;
private static final int INITIAL_DELAY_MS = 500;
private static final double DELAY_SCALE_FACTOR = 2;
/**
- *
- * @param segmentName
- * @param downloadScheme Can be either http or https.
- * @param helixManager
- * @return a list of uri strings of the form
http(s)://hostname:port/segments/tablenameWithType/segmentName
- * for the servers hosting ONLINE segments; empty list if no such server
found.
+ * Returns a list of URIs of the form
'http(s)://hostname:port/segments/tableNameWithType/segmentName' for the servers
+ * hosting ONLINE segments; empty list if no such server found. The download
scheme can be either 'http' or 'https'.
*/
- public static List<URI> getPeerServerURIs(String segmentName, String
downloadScheme, HelixManager helixManager) {
- LLCSegmentName llcSegmentName = new LLCSegmentName(segmentName);
- String tableNameWithType =
-
TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(llcSegmentName.getTableName());
- return getPeerServerURIs(segmentName, downloadScheme, helixManager,
tableNameWithType);
- }
-
- public static List<URI> getPeerServerURIs(String segmentName, String
downloadScheme,
- HelixManager helixManager, String tableNameWithType) {
+ public static List<URI> getPeerServerURIs(HelixManager helixManager, String
tableNameWithType, String segmentName,
+ String downloadScheme) {
HelixAdmin helixAdmin = helixManager.getClusterManagmentTool();
String clusterName = helixManager.getClusterName();
- if (clusterName == null) {
- _logger.error("ClusterName not found");
- return ListUtils.EMPTY_LIST;
- }
- final List<URI> onlineServerURIs = new ArrayList<>();
+ List<URI> onlineServerURIs = new ArrayList<>();
try {
RetryPolicies.exponentialBackoffRetryPolicy(MAX_NUM_ATTEMPTS,
INITIAL_DELAY_MS, DELAY_SCALE_FACTOR)
.attempt(() -> {
- getOnlineServersFromExternalView(segmentName, downloadScheme,
tableNameWithType, helixAdmin, clusterName,
+ getOnlineServersFromExternalView(helixAdmin, clusterName,
tableNameWithType, segmentName, downloadScheme,
onlineServerURIs);
return !onlineServerURIs.isEmpty();
});
+ } catch (AttemptsExceededException e) {
+ LOGGER.error("Failed to find ONLINE servers for segment: {}, table: {}",
segmentName, tableNameWithType);
Review Comment:
nit: `"Failed to ... in {} attempts", ..., MAX_NUM_ATTEMPTS)` to make error
msg a bit more clear
--
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
