[
https://issues.apache.org/jira/browse/HADOOP-18694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17714730#comment-17714730
]
ASF GitHub Bot commented on HADOOP-18694:
-----------------------------------------
ayushtkn commented on code in PR #5542:
URL: https://github.com/apache/hadoop/pull/5542#discussion_r1173022015
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:
##########
@@ -590,7 +590,7 @@ private synchronized boolean updateAddress() throws
IOException {
InetSocketAddress currentAddr = NetUtils.createSocketAddrForHost(
server.getHostName(), server.getPort());
- if (!server.equals(currentAddr)) {
+ if (!currentAddr.isUnresolved() && !server.equals(currentAddr)) {
LOG.warn("Address change detected. Old: " + server.toString() +
" New: " + currentAddr.toString());
Review Comment:
Since you are touching this part can you fix this log line as well?
```
LOG.warn("Address change detected. Old: {} New: {}", server,
currentAddr);
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java:
##########
@@ -1728,6 +1728,55 @@ public void testProxyUserBinding() throws Exception {
checkUserBinding(true);
}
+ @Test(timeout=60000)
+ public void testUpdateAddressEnsureResolved() throws Exception {
+ // start server
+ Server server = new TestServer(5, false);
+ server.start();
+
+ SocketFactory mockFactory = Mockito.mock(SocketFactory.class);
+ doThrow(new
ConnectTimeoutException("fake")).when(mockFactory).createSocket();
+ Client client = new Client(LongWritable.class, conf, mockFactory);
+ InetSocketAddress address = new InetSocketAddress("localhost", 9090);
+ ConnectionId remoteId = getConnectionId(address, 100, conf);
+ try {
+ LambdaTestUtils.intercept(IOException.class,
+ new Callable<Void>() {
+ @Override
+ public Void call() throws IOException {
+ client.call(RpcKind.RPC_BUILTIN,
+ new LongWritable(RANDOM.nextLong()), remoteId,
+ RPC.RPC_SERVICE_CLASS_DEFAULT, null);
+ return null;
+ }
+ });
+
+ assertFalse(address.isUnresolved());
+ assertFalse(remoteId.getAddress().isUnresolved());
+ assertEquals(System.identityHashCode(remoteId.getAddress()),
+ System.identityHashCode(address));
+
+ NetUtils.addStaticResolution("localhost", "host.invalid");
+ LambdaTestUtils.intercept(IOException.class,
+ new Callable<Void>() {
+ @Override
+ public Void call() throws IOException {
+ client.call(RpcKind.RPC_BUILTIN,
+ new LongWritable(RANDOM.nextLong()), remoteId,
+ RPC.RPC_SERVICE_CLASS_DEFAULT, null);
+ return null;
+ }
+ });
Review Comment:
Use lambda
```
LambdaTestUtils.intercept(IOException.class, (Callable<Void>) () -> {
client.call(RpcKind.RPC_BUILTIN, new
LongWritable(RANDOM.nextLong()), remoteId,
RPC.RPC_SERVICE_CLASS_DEFAULT, null);
return null;
});
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java:
##########
@@ -1728,6 +1728,55 @@ public void testProxyUserBinding() throws Exception {
checkUserBinding(true);
}
+ @Test(timeout=60000)
+ public void testUpdateAddressEnsureResolved() throws Exception {
+ // start server
+ Server server = new TestServer(5, false);
+ server.start();
+
+ SocketFactory mockFactory = Mockito.mock(SocketFactory.class);
+ doThrow(new
ConnectTimeoutException("fake")).when(mockFactory).createSocket();
+ Client client = new Client(LongWritable.class, conf, mockFactory);
+ InetSocketAddress address = new InetSocketAddress("localhost", 9090);
+ ConnectionId remoteId = getConnectionId(address, 100, conf);
+ try {
+ LambdaTestUtils.intercept(IOException.class,
+ new Callable<Void>() {
+ @Override
+ public Void call() throws IOException {
+ client.call(RpcKind.RPC_BUILTIN,
+ new LongWritable(RANDOM.nextLong()), remoteId,
+ RPC.RPC_SERVICE_CLASS_DEFAULT, null);
+ return null;
+ }
+ });
Review Comment:
Use lambda
```
LambdaTestUtils.intercept(IOException.class, (Callable<Void>) () -> {
client.call(RpcKind.RPC_BUILTIN, new
LongWritable(RANDOM.nextLong()), remoteId,
RPC.RPC_SERVICE_CLASS_DEFAULT, null);
return null;
});
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java:
##########
@@ -1728,6 +1728,55 @@ public void testProxyUserBinding() throws Exception {
checkUserBinding(true);
}
+ @Test(timeout=60000)
+ public void testUpdateAddressEnsureResolved() throws Exception {
+ // start server
+ Server server = new TestServer(5, false);
Review Comment:
Why do you need 5, 1 should work as well?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java:
##########
@@ -1728,6 +1728,55 @@ public void testProxyUserBinding() throws Exception {
checkUserBinding(true);
}
+ @Test(timeout=60000)
+ public void testUpdateAddressEnsureResolved() throws Exception {
+ // start server
+ Server server = new TestServer(5, false);
+ server.start();
+
+ SocketFactory mockFactory = Mockito.mock(SocketFactory.class);
+ doThrow(new
ConnectTimeoutException("fake")).when(mockFactory).createSocket();
+ Client client = new Client(LongWritable.class, conf, mockFactory);
+ InetSocketAddress address = new InetSocketAddress("localhost", 9090);
Review Comment:
Does 9090 has any relevance here? If the port is occupied in the env where
test run, will there be any impact? If yes, then use
```NetUtils.getFreeSocketPort()```
> Client.Connection#updateAddress needs to ensure that address is resolved
> before updating
> ----------------------------------------------------------------------------------------
>
> Key: HADOOP-18694
> URL: https://issues.apache.org/jira/browse/HADOOP-18694
> Project: Hadoop Common
> Issue Type: Improvement
> Components: common
> Affects Versions: 3.4.0, 3.3.5
> Reporter: dzcxzl
> Priority: Major
> Labels: pull-request-available
>
> When Client.Connection#setupConnection encounters an IOException, it will try
> to update the server address.
> ([HADOOP-18365|https://issues.apache.org/jira/browse/HADOOP-18365])
> When the address is re-parsed, it may be an unresolved address
> (UnknownHostException), which causes Client.Connection#setupConnection to
> fail to reconnect.
> {code:java}
> while (true) {
> try {
> if (server.isUnresolved()) { {code}
> Especially when DN is connected to NN, BPServiceActor#bpNamenode is only
> initialized once, which causes DN to never connect to NN before restarting.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
