The test sets up two datanodes and creates a file. Then it wait for NumOpenConnections to reach 2. According to the log, registerDatanode() was called twice, one for each datanode.
Both datanodes are receiving blocks and sending IBRs. But when it checks for NumOpenConnections, it stays at 1. The test times out and fails.

It looks like an issue in the metrics.

Attaching the test log. This is from a 2.7 build but fails the same way.

I found that it fails when I run the two tests together so testDataNode() runs before testNameNode(). If I run only testNameNode or I change the order the tests pass.

I debugged a bit the failed run. It opens two RPC connections but later on it closes one of them in Server class doRead(SelectionKey key) method:

  if (count < 0) {
    closeConnection(c);
    c = null;
  }

Here the count is -1. We get -1 in Server:channelRead(ReadableByteChannel channel, ByteBuffer buffer) method:

int count = (buffer.remaining() <= NIO_BUFFER_LIMIT) ?
                channel.read(buffer) : channelIO(channel, null, buffer);

In this condition the true branch will run.

Can somebody help us on this?

We observed that the fail is caused by this assert:

DFSTestUtil.waitForMetric(jmx, "NumOpenConnections", numDatanodes);

This checks if the number of open connections equals to the number of data nodes. But the number of open connections has absolutely no dependency from the data nodes: it's either 0, 1 (DataNodeProtocol) or 2 (DataNodeProtocol and ClientProtocol). The test passes in those rare cases when the ClientProtocol hasn't timed out when the TestNameNode runs (this can only happen if the tests are run separately). If we increment the number of data nodes (to 3, or so) the test will always fail. Contrarily if we increase the client timeout (ipc.client.connection.maxidletime) the test will always pass.

Our suggestion is to remove this assert.

-1 overall

This message was automatically generated.

It looks like a race was introduced after the jmx caching was "fixed". Certain metrics values in JMX is now using the cached value, which expires in 10 seconds by default. Since the ipc client idle timeout is also 10 seconds, The ipc connection may get closed before the jmx refresh. So the number of actual connections reaches to 2, but can drop to 1 just before a new jmx data is fetched. We could fix this by either lowering the jmx cache expiration or increasing ipc client idle timeout. Since it is a JMX test, it might be better to leave the jmx setting as default and change the ipc timeout to, say, 15 seconds.

So Kihwal Lee, are you suggesting to have a wait/assert for checking if the number of connections is equal to 2 (datanode, client)? And to fulfil this to increase the ipc client idle timeout?
If so, can you (or the original author of the test) explain why we need to test if the client is still connected?

It is not for testing the client connection being up. It is simply checking one of the metrics values reported in JMX. I don't know the reason why NumOpenConnections was chosen. The test had worked reliably until the jmx caching was fixed. The values used to be available right away, but now it takes about 10 seconds. So when it's working it adds about 10 more seconds of delay.

But the original author also made a wrong assumption. The assumption was that the reason for the number of connections being 2 is due to having two datanodes. As you have correctly analyzed, this is not true in a MiniDFSCluster. Since the two datanodes are sharing the same JVM, a single connection was shared for the DatanodeProtocol. An additional connection was made for the client. In a real distributed cluster, it would have been 3 connections.

I lean toward fixing the existing check than removing it. First it shouldn't check against the number of datanods, but simply 2. Regarding increasing ipc client idle timeout, it will make test run time longer, which is against what we have been trying to do. An alternative is to add a test resource to reduce the jmx update interval. We could add a hadoop-hdfs-project/hadoop-hdfs/src/test/resources/hadoop-metrics2.properties file with one line containing *.period=1. This will also reduce the run time of a number of test cases that query jmx to verify the result.

What do you think?

If adding a new test resource, don't forget to add it to the exclude list for apache-rat-plugin.

Hi Kihwal Lee,

We do not see the value to check NumOpenConnections since it can cause intermittent failures and with the other checks the JMX metrics are tested enough. Our suggestion is to remove NumOpenConnections check. Do you see any disadvantages removing NumOpenConnections check?

In addition playing with timeouts makes a JUnit test more complicated than a JUnit test should be. What is you opinion about these?

In addition playing with timeouts makes a JUnit test more complicated than a JUnit test should be. What is you opinion about these?

Totally agree, but in reality hadoop unit tests are riddled with unreliable timeouts. Many of them are not really unit tests. They exist because of many reasons including Inherent difficulties in unit testing distributed functionalities, test unfriendly designs and simply bad test designs. Many are aware of the issue.

Do you see any disadvantages removing NumOpenConnections check?

I was trying to keep whatever the original author did, but in term of the value of the check, I think it is fine to remove. So +1 for the patch.

Committed it to trunk, branch-2 and branch-2.8. Thanks for fixing this, Gergely.

FAILURE: Integrated in Hadoop-trunk-Commit #9603 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9603/)
HDFS-10270. TestJMXGet:testNameNode() fails. Contributed by Gergely (kihwal: rev d2f3bbc29046435904ad9418073795439c71b441)

• hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/tools/TestJMXGet.java

I just committed the patch to branch-2.7 as part of an effort of backporting HDFS-7933. The patch could only possibly affect TestJMXGet and I verified locally that it passes fine with the patch; the patch actually only makes the test easier to pass.