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
I split the host comparison so that case-insensitive matching applies only
when neither side is an IP literal, and also added a test. 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]
