chihsuan commented on code in PR #10807:
URL: https://github.com/apache/ozone/pull/10807#discussion_r3766327139
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java:
##########
@@ -249,24 +252,76 @@ public synchronized void
performFailoverToAssignedLeader(String newLeader,
Exception e) {
ServerNotLeaderException snle =
(ServerNotLeaderException) SCMHAUtils.getServerNotLeaderException(e);
- if (snle != null && snle.getSuggestedLeader() != null) {
- Optional<SCMProxyInfo> matchedProxyInfo =
- scmProxyInfoMap.values().stream().filter(
- proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress())
- .equals(snle.getSuggestedLeader())).findFirst();
- if (matchedProxyInfo.isPresent()) {
- newLeader = matchedProxyInfo.get().getNodeId();
+ String suggestedLeader = snle != null ? snle.getSuggestedLeader() : null;
+ if (suggestedLeader != null) {
+ Optional<String> matchedNodeId =
findSuggestedLeaderNodeId(suggestedLeader);
+ if (matchedNodeId.isPresent()) {
+ newLeader = matchedNodeId.get();
getLogger().debug("Performing failover to suggested leader {}, nodeId
{}",
- snle.getSuggestedLeader(), newLeader);
+ suggestedLeader, newLeader);
} else {
getLogger().debug("Suggested leader {} does not match with any of the
" +
- "proxyInfo address {}", snle.getSuggestedLeader(),
+ "proxyInfo address {}", suggestedLeader,
Arrays.toString(scmProxyInfoMap.values().toArray()));
}
}
assignLeaderToNode(newLeader);
}
+ /**
+ * Find the SCM nodeId whose address matches the suggested-leader authority.
Callers hold
+ * the provider monitor, so this must never resolve a name.
+ */
+ private Optional<String> findSuggestedLeaderNodeId(String suggestedLeader) {
+ final Optional<String> host;
+ final OptionalInt port;
+ try {
+ host = HddsUtils.getHostName(suggestedLeader);
+ port = HddsUtils.getHostPort(suggestedLeader);
+ } catch (IllegalArgumentException ex) {
+ getLogger().warn("Ignoring unparseable suggested leader {}",
suggestedLeader, ex);
+ return Optional.empty();
+ }
+ if (!host.isPresent() || !port.isPresent()) {
+ return Optional.empty();
+ }
+ return scmProxyInfoMap.values().stream()
+ .filter(proxyInfo -> matchesAuthority(proxyInfo.getAddress(),
host.get(), port.getAsInt()))
+ .findFirst()
+ .map(SCMProxyInfo::getNodeId);
+ }
+
+ private static boolean matchesAuthority(InetSocketAddress address, String
host, int port) {
+ if (address.getPort() != port) {
+ return false;
+ }
+ // Both getHostString() and getAddress() read what was resolved when the
proxy info was
+ // built, so neither triggers a lookup here.
+ String addressHost = address.getHostString();
+ if (addressHost.equalsIgnoreCase(host) || sameIpLiteral(addressHost,
host)) {
Review Comment:
Good catch. fixed in
https://github.com/apache/ozone/pull/10807/commits/b7390ab147dccb36cbb385a000f67d2c620202bb
Thanks!
--
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]