adoroszlai commented on code in PR #9997:
URL: https://github.com/apache/ozone/pull/9997#discussion_r3031846821
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java:
##########
@@ -739,30 +738,13 @@ private void decreasePendingMetricsAndReleaseSemaphore() {
private void checkOpen(DatanodeDetails dn)
throws IOException {
- if (isClosed.get()) {
- throw new IOException("This channel is not connected.");
- }
-
- ManagedChannel channel = channels.get(dn.getID());
- // If the channel doesn't exist for this specific datanode or the channel
- // is closed, just reconnect
- if (!isConnected(channel)) {
- reconnect(dn);
- }
-
- }
-
- private void reconnect(DatanodeDetails dn)
- throws IOException {
- ManagedChannel channel;
try {
connectToDatanode(dn);
- channel = channels.get(dn.getID());
} catch (Exception e) {
throw new IOException("Error while connecting", e);
}
Review Comment:
With simplified `checkOpen`, there are two test cases that need to be
updated, sorry for missing that.
```
Expecting actual:
"Error while connecting"
to contain:
"This channel is not connected"
at
org.apache.hadoop.hdds.scm.TestXceiverClientManager.testFreeByReference(TestXceiverClientManager.java:162)
...
at
org.apache.hadoop.hdds.scm.TestXceiverClientManager.testFreeByEviction(TestXceiverClientManager.java:211)
```
Let's remove this `try-catch`, we don't need to wrap the exception from
`connectToDatanode`.
With that, exception will have the correct message (`Client is closed`).
Please update the test to reflect that.
https://github.com/apache/ozone/blob/8fbe16b4efaa1e40b90d890028e447317cae3d01/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java#L162
https://github.com/apache/ozone/blob/8fbe16b4efaa1e40b90d890028e447317cae3d01/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java#L211
```diff
diff --git
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
index a9dbcb9456..1f9ac0a122 100644
---
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
+++
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
@@ -738,11 +738,7 @@ private void
decreasePendingMetricsAndReleaseSemaphore() {
private void checkOpen(DatanodeDetails dn)
throws IOException {
- try {
- connectToDatanode(dn);
- } catch (Exception e) {
- throw new IOException("Error while connecting", e);
- }
+ connectToDatanode(dn);
if (!isConnected(dn)) {
throw new IOException("This channel is not connected.");
diff --git
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java
index 06d19e5575..9468cec94f 100644
---
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java
+++
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManager.java
@@ -159,7 +159,7 @@ public void testFreeByReference(@TempDir Path metaDir)
throws IOException {
Throwable t = assertThrows(IOException.class,
() -> ContainerProtocolCalls.createContainer(client1,
container1.getContainerInfo().getContainerID(), null));
- assertThat(t.getMessage()).contains("This channel is not connected");
+ assertThat(t.getMessage()).contains("Client is closed");
clientManager.releaseClient(client2, false);
}
@@ -208,7 +208,7 @@ public void testFreeByEviction(@TempDir Path metaDir)
throws IOException {
Throwable t = assertThrows(IOException.class,
() -> ContainerProtocolCalls.createContainer(client1,
container1.getContainerInfo().getContainerID(), null));
- assertThat(t.getMessage()).contains("This channel is not connected");
+ assertThat(t.getMessage()).contains("Client is closed");
clientManager.releaseClient(client2, false);
}
```
--
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]
