smengcl commented on code in PR #10807:
URL: https://github.com/apache/ozone/pull/10807#discussion_r3764590629
##########
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:
This shortcut also compares an IPv6 zone name case-insensitively. With
otherwise identical entries at `%ETH0` and `%eth0`, the `%eth0` hint selects
the `%ETH0` SCM before `sameIpLiteral` can compare the scopes as text.
Interface names are case-sensitive, so this can still pin failover to the wrong
node. Please apply case-insensitive matching only to hostnames, or try the
literal-aware comparison first, and add zone-name coverage.
--
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]
