From 02681f2aeccb7365c58c9e5a0f909ab38da0b762 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 15:35:55 +0530 Subject: [PATCH 1/9] HDDS-5152. Fix Suggested leader in Client. --- .../hdds/ratis/ServerNotLeaderException.java | 5 +- .../ratis/TestServerNotLeaderException.java | 56 +++++++++++++++++++ ...SCMBlockLocationFailoverProxyProvider.java | 18 ++++-- 3 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java index 71b26f587882..eaf1f9bec070 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java @@ -33,7 +33,8 @@ public class ServerNotLeaderException extends IOException { private static final Pattern CURRENT_PEER_ID_PATTERN = Pattern.compile("Server:(.*) is not the leader[.]+.*", Pattern.DOTALL); private static final Pattern SUGGESTED_LEADER_PATTERN = - Pattern.compile(".*Suggested leader is Server:([^.]*).*", Pattern.DOTALL); + Pattern.compile(".*Suggested leader is Server:([^:]*)(:[0-9]+).*", + Pattern.DOTALL); public ServerNotLeaderException(RaftPeerId currentPeerId) { super("Server:" + currentPeerId + " is not the leader. Could not " + @@ -60,7 +61,7 @@ public ServerNotLeaderException(String message) { Matcher suggestedLeaderMatcher = SUGGESTED_LEADER_PATTERN.matcher(message); if (suggestedLeaderMatcher.matches()) { - this.leader = suggestedLeaderMatcher.group(1); + this.leader = suggestedLeaderMatcher.group(1) + suggestedLeaderMatcher.group(2); } else { this.leader = null; } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java new file mode 100644 index 000000000000..1b7275e48d4a --- /dev/null +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.ratis; + +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; + +public class TestServerNotLeaderException { + @Test + public void testServerNotLeaderException() throws IOException { + + // Test hostname with "." + final String msg = + "Server:cf0bc565-a41b-4784-a24d-3048d5a5b013 is not the leader. " + + "Suggested leader is Server:scm5-3.scm5.root.hwx.site:9863"; + ServerNotLeaderException snle = new ServerNotLeaderException(msg); + Assert.assertEquals(snle.getSuggestedLeader(), "scm5-3.scm5.root.hwx" + + ".site:9863"); + + String message = "Server:7fdd7170-75cc-4e11-b343-c2657c2f2f39 is not the " + + "leader.Suggested leader is Server:scm5-3.scm5.root.hwx.site:9863 \n" + + "at org.apache.hadoop.hdds.ratis.ServerNotLeaderException" + + ".convertToNotLeaderException(ServerNotLeaderException.java:96)"; + snle = new ServerNotLeaderException(message); + Assert.assertEquals("scm5-3.scm5.root.hwx.site:9863", + snle.getSuggestedLeader()); + + // Test hostname with out "." + message = "Server:7fdd7170-75cc-4e11-b343-c2657c2f2f39 is not the " + + "leader.Suggested leader is Server:localhost:98634 \n" + + "at org.apache.hadoop.hdds.ratis.ServerNotLeaderException" + + ".convertToNotLeaderException(ServerNotLeaderException.java:96)"; + snle = new ServerNotLeaderException(message); + Assert.assertEquals("localhost:98634", + snle.getSuggestedLeader()); + } + +} diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java index b0188facea26..9a94099bb3bc 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java @@ -41,9 +41,11 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import static org.apache.hadoop.ozone.OzoneConsts.SCM_DUMMY_SERVICE_ID; @@ -161,11 +163,19 @@ public synchronized void performFailoverToAssignedLeader(String newLeader, ServerNotLeaderException snle = (ServerNotLeaderException) SCMHAUtils.getServerNotLeaderException(e); if (snle != null && snle.getSuggestedLeader() != null) { - newLeader = scmProxyInfoMap.values().stream().filter( + Optional matchedProxyInfo = + scmProxyInfoMap.values().stream().filter( proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) - .equals(snle.getSuggestedLeader())).findFirst().get().getNodeId(); - LOG.debug("Performing failover to suggested leader {}, nodeId {}", - snle.getSuggestedLeader(), newLeader); + .equals(snle.getSuggestedLeader())).findFirst(); + if (matchedProxyInfo.isPresent()) { + newLeader = matchedProxyInfo.get().getNodeId(); + LOG.debug("Performing failover to suggested leader {}, nodeId {}", + snle.getSuggestedLeader(), newLeader); + } else { + LOG.debug("Suggested leader {} does not match with any of the " + + "proxyInfo adress {}", snle.getSuggestedLeader(), + Arrays.toString(scmProxyInfoMap.values().toArray())); + } } if (newLeader == null) { // If newLeader is not assigned, it will fail over to next proxy. From 382d2b6e7c44b1321f2aa60693305acc141c9711 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 15:46:33 +0530 Subject: [PATCH 2/9] fix other failoverproxy --- ...ontainerLocationFailoverProxyProvider.java | 20 +++++++++++++----- ...SecurityProtocolFailoverProxyProvider.java | 21 ++++++++++++++----- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java index 96e627555535..1cff58a87267 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java @@ -41,9 +41,11 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; /** @@ -167,11 +169,19 @@ public synchronized void performFailoverToAssignedLeader(String newLeader, ServerNotLeaderException snle = (ServerNotLeaderException) SCMHAUtils.getServerNotLeaderException(e); if (snle != null && snle.getSuggestedLeader() != null) { - newLeader = scmProxyInfoMap.values().stream().filter( - proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) - .equals(snle.getSuggestedLeader())).findFirst().get().getNodeId(); - LOG.debug("Performing failover to suggested leader {}, nodeId {}", - snle.getSuggestedLeader(), newLeader); + Optional matchedProxyInfo = + scmProxyInfoMap.values().stream().filter( + proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) + .equals(snle.getSuggestedLeader())).findFirst(); + if (matchedProxyInfo.isPresent()) { + newLeader = matchedProxyInfo.get().getNodeId(); + LOG.debug("Performing failover to suggested leader {}, nodeId {}", + snle.getSuggestedLeader(), newLeader); + } else { + LOG.debug("Suggested leader {} does not match with any of the " + + "proxyInfo adress {}", snle.getSuggestedLeader(), + Arrays.toString(scmProxyInfoMap.values().toArray())); + } } if (newLeader == null) { // If newLeader is not assigned, it will fail over to next proxy. diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java index 23f3a3e6cbea..007db27d61a5 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java @@ -41,9 +41,11 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; /** * Failover proxy provider for SCMSecurityProtocol server. @@ -187,11 +189,20 @@ public synchronized void performFailoverToAssignedLeader(String newLeader, ServerNotLeaderException snle = (ServerNotLeaderException) SCMHAUtils.getServerNotLeaderException(e); if (snle != null && snle.getSuggestedLeader() != null) { - newLeader = scmProxyInfoMap.values().stream().filter( - proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) - .equals(snle.getSuggestedLeader())).findFirst().get().getNodeId(); - LOG.debug("Performing failover to suggested leader {}, nodeId {}", - snle.getSuggestedLeader(), newLeader); + if (snle != null && snle.getSuggestedLeader() != null) { + Optional matchedProxyInfo = + scmProxyInfoMap.values().stream().filter( + proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) + .equals(snle.getSuggestedLeader())).findFirst(); + if (matchedProxyInfo.isPresent()) { + newLeader = matchedProxyInfo.get().getNodeId(); + LOG.debug("Performing failover to suggested leader {}, nodeId {}", + snle.getSuggestedLeader(), newLeader); + } else { + LOG.debug("Suggested leader {} does not match with any of the " + + "proxyInfo adress {}", snle.getSuggestedLeader(), + Arrays.toString(scmProxyInfoMap.values().toArray())); + } } if (newLeader == null) { // If newLeader is not assigned, it will fail over to next proxy. From 0fc375b76d45dd419cc0c1cfb1b2d7ab0099c3db Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 15:48:20 +0530 Subject: [PATCH 3/9] a --- ...SecurityProtocolFailoverProxyProvider.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java index 007db27d61a5..2e49b61eb55d 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java @@ -189,20 +189,19 @@ public synchronized void performFailoverToAssignedLeader(String newLeader, ServerNotLeaderException snle = (ServerNotLeaderException) SCMHAUtils.getServerNotLeaderException(e); if (snle != null && snle.getSuggestedLeader() != null) { - if (snle != null && snle.getSuggestedLeader() != null) { - Optional matchedProxyInfo = - scmProxyInfoMap.values().stream().filter( - proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) - .equals(snle.getSuggestedLeader())).findFirst(); - if (matchedProxyInfo.isPresent()) { - newLeader = matchedProxyInfo.get().getNodeId(); - LOG.debug("Performing failover to suggested leader {}, nodeId {}", - snle.getSuggestedLeader(), newLeader); - } else { - LOG.debug("Suggested leader {} does not match with any of the " + - "proxyInfo adress {}", snle.getSuggestedLeader(), - Arrays.toString(scmProxyInfoMap.values().toArray())); - } + Optional< SCMProxyInfo > matchedProxyInfo = + scmProxyInfoMap.values().stream().filter( + proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) + .equals(snle.getSuggestedLeader())).findFirst(); + if (matchedProxyInfo.isPresent()) { + newLeader = matchedProxyInfo.get().getNodeId(); + LOG.debug("Performing failover to suggested leader {}, nodeId {}", + snle.getSuggestedLeader(), newLeader); + } else { + LOG.debug("Suggested leader {} does not match with any of the " + + "proxyInfo adress {}", snle.getSuggestedLeader(), + Arrays.toString(scmProxyInfoMap.values().toArray())); + } } if (newLeader == null) { // If newLeader is not assigned, it will fail over to next proxy. From 037d11c682db78e91b08db170f61fdb7ec3b49d7 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 15:52:39 +0530 Subject: [PATCH 4/9] fix other proxy provider and fix cs --- .../apache/hadoop/hdds/ratis/ServerNotLeaderException.java | 3 ++- .../hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java index eaf1f9bec070..147047286a27 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java @@ -61,7 +61,8 @@ public ServerNotLeaderException(String message) { Matcher suggestedLeaderMatcher = SUGGESTED_LEADER_PATTERN.matcher(message); if (suggestedLeaderMatcher.matches()) { - this.leader = suggestedLeaderMatcher.group(1) + suggestedLeaderMatcher.group(2); + this.leader = suggestedLeaderMatcher.group(1) + + suggestedLeaderMatcher.group(2); } else { this.leader = null; } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java index 9a94099bb3bc..70787407919c 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java @@ -165,8 +165,8 @@ public synchronized void performFailoverToAssignedLeader(String newLeader, if (snle != null && snle.getSuggestedLeader() != null) { Optional matchedProxyInfo = scmProxyInfoMap.values().stream().filter( - proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) - .equals(snle.getSuggestedLeader())).findFirst(); + proxyInfo -> NetUtils.getHostPortString(proxyInfo.getAddress()) + .equals(snle.getSuggestedLeader())).findFirst(); if (matchedProxyInfo.isPresent()) { newLeader = matchedProxyInfo.get().getNodeId(); LOG.debug("Performing failover to suggested leader {}, nodeId {}", From f375d58946ea080dc521656adc22e1dbb8d40014 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 15:58:49 +0530 Subject: [PATCH 5/9] add safety check of groupcount --- .../hadoop/hdds/ratis/ServerNotLeaderException.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java index 147047286a27..464019278afa 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java @@ -61,8 +61,12 @@ public ServerNotLeaderException(String message) { Matcher suggestedLeaderMatcher = SUGGESTED_LEADER_PATTERN.matcher(message); if (suggestedLeaderMatcher.matches()) { - this.leader = suggestedLeaderMatcher.group(1) + - suggestedLeaderMatcher.group(2); + if (suggestedLeaderMatcher.groupCount() == 2) { + this.leader = suggestedLeaderMatcher.group(1) + + suggestedLeaderMatcher.group(2); + } else { + this.leader = null; + } } else { this.leader = null; } From 9438a2c0129df33cdc7f37b6349524dfb8dbb238 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 16:39:26 +0530 Subject: [PATCH 6/9] fix fb --- hadoop-hdds/common/pom.xml | 5 +++++ .../hadoop/hdds/ratis/TestServerNotLeaderException.java | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/common/pom.xml b/hadoop-hdds/common/pom.xml index 10e5c3209b01..cebee64d1409 100644 --- a/hadoop-hdds/common/pom.xml +++ b/hadoop-hdds/common/pom.xml @@ -200,6 +200,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> hamcrest-all test + + com.github.spotbugs + spotbugs-annotations + test + diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java index 1b7275e48d4a..ad35abf4e841 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java @@ -18,14 +18,18 @@ package org.apache.hadoop.hdds.ratis; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Assert; import org.junit.Test; import java.io.IOException; +/** Class to test {@link ServerNotLeaderException} parsing. **/ + +@SuppressFBWarnings("NM_CLASS_NOT_EXCEPTION") public class TestServerNotLeaderException { @Test - public void testServerNotLeaderException() throws IOException { + public void testServerNotLeaderException() { // Test hostname with "." final String msg = From b611f2405a96ba386fe9de486072154a35c5c75d Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 16:41:38 +0530 Subject: [PATCH 7/9] fix cs --- .../apache/hadoop/hdds/ratis/TestServerNotLeaderException.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java index ad35abf4e841..63ed362a1c8b 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java @@ -22,8 +22,6 @@ import org.junit.Assert; import org.junit.Test; -import java.io.IOException; - /** Class to test {@link ServerNotLeaderException} parsing. **/ @SuppressFBWarnings("NM_CLASS_NOT_EXCEPTION") From b220e0ca90050cb6d2fa7dc65623653ea2f7efa3 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 17:48:40 +0530 Subject: [PATCH 8/9] fix review comments --- .../hadoop/hdds/ratis/ServerNotLeaderException.java | 10 ++++++++-- .../hdds/ratis/TestServerNotLeaderException.java | 8 ++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java index 464019278afa..ee4504232f0a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java @@ -60,10 +60,16 @@ public ServerNotLeaderException(String message) { Matcher suggestedLeaderMatcher = SUGGESTED_LEADER_PATTERN.matcher(message); + // TODO: If message contains only port or multiple ":" this will return + // suggested leader which will not be a valid host. if (suggestedLeaderMatcher.matches()) { if (suggestedLeaderMatcher.groupCount() == 2) { - this.leader = suggestedLeaderMatcher.group(1) + - suggestedLeaderMatcher.group(2); + if (suggestedLeaderMatcher.group(1).equals("")) { + this.leader = null; + } else { + this.leader = suggestedLeaderMatcher.group(1) + + suggestedLeaderMatcher.group(2); + } } else { this.leader = null; } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java index 63ed362a1c8b..8ad346aeb617 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java @@ -53,6 +53,14 @@ public void testServerNotLeaderException() { snle = new ServerNotLeaderException(message); Assert.assertEquals("localhost:98634", snle.getSuggestedLeader()); + + message = "Server:7fdd7170-75cc-4e11-b343-c2657c2f2f39 is not the " + + "leader.Suggested leader is Server::98634 \n" + + "at org.apache.hadoop.hdds.ratis.ServerNotLeaderException" + + ".convertToNotLeaderException(ServerNotLeaderException.java:96)"; + snle = new ServerNotLeaderException(message); + Assert.assertEquals(null, + snle.getSuggestedLeader()); } } From 0269c3673adfdd2278d08e82644f1be53b3a3da7 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Tue, 27 Apr 2021 18:00:29 +0530 Subject: [PATCH 9/9] fix review comments --- .../hdds/ratis/ServerNotLeaderException.java | 5 ++--- .../hdds/ratis/TestServerNotLeaderException.java | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java index ee4504232f0a..d100b4a0490a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java @@ -60,11 +60,10 @@ public ServerNotLeaderException(String message) { Matcher suggestedLeaderMatcher = SUGGESTED_LEADER_PATTERN.matcher(message); - // TODO: If message contains only port or multiple ":" this will return - // suggested leader which will not be a valid host. if (suggestedLeaderMatcher.matches()) { if (suggestedLeaderMatcher.groupCount() == 2) { - if (suggestedLeaderMatcher.group(1).equals("")) { + if (suggestedLeaderMatcher.group(1).isEmpty() + || suggestedLeaderMatcher.group(2).isEmpty()) { this.leader = null; } else { this.leader = suggestedLeaderMatcher.group(1) + diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java index 8ad346aeb617..3bdcf85cdf7c 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestServerNotLeaderException.java @@ -61,6 +61,22 @@ public void testServerNotLeaderException() { snle = new ServerNotLeaderException(message); Assert.assertEquals(null, snle.getSuggestedLeader()); + + message = "Server:7fdd7170-75cc-4e11-b343-c2657c2f2f39 is not the " + + "leader.Suggested leader is Server:localhost:98634:8988 \n" + + "at org.apache.hadoop.hdds.ratis.ServerNotLeaderException" + + ".convertToNotLeaderException(ServerNotLeaderException.java:96)"; + snle = new ServerNotLeaderException(message); + Assert.assertEquals("localhost:98634", + snle.getSuggestedLeader()); + + message = "Server:7fdd7170-75cc-4e11-b343-c2657c2f2f39 is not the " + + "leader.Suggested leader is Server:localhost \n" + + "at org.apache.hadoop.hdds.ratis.ServerNotLeaderException" + + ".convertToNotLeaderException(ServerNotLeaderException.java)"; + snle = new ServerNotLeaderException(message); + Assert.assertEquals(null, + snle.getSuggestedLeader()); } }