You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2020/04/03 03:31:18 UTC
[hbase] branch master updated: HBASE-24107 [Flakey Test]
TestThriftServerCmdLine.testRunThriftServer NPEs if InfoServer port clash
This is an automated email from the ASF dual-hosted git repository.
stack pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new edca9c3 HBASE-24107 [Flakey Test] TestThriftServerCmdLine.testRunThriftServer NPEs if InfoServer port clash
edca9c3 is described below
commit edca9c3a0ef787e1f57e8524bdb1eb8b2bdb1d02
Author: stack <st...@apache.org>
AuthorDate: Thu Apr 2 20:29:58 2020 -0700
HBASE-24107 [Flakey Test] TestThriftServerCmdLine.testRunThriftServer NPEs if InfoServer port clash
---
.../hbase/thrift/TestBindExceptionHandling.java | 21 +++++-
.../hbase/thrift/TestThriftServerCmdLine.java | 80 ++++++++++++++++------
2 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java
index fa1d141..9fc9542 100644
--- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java
+++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java
@@ -32,14 +32,29 @@ public class TestBindExceptionHandling {
HBaseClassTestRule.forClass(TestBindExceptionHandling.class);
/**
- * See if random port choosing works around port clashes
+ * See if random port choosing works around protocol port clashes
*/
@Test
- public void testPortClash() {
+ public void testProtocolPortClash() {
ThriftServer thriftServer = null;
try {
thriftServer = new TestThriftServerCmdLine(null, false, false, false).
- createBoundServer(true);
+ createBoundServer(true, false);
+ assertNotNull(thriftServer.tserver);
+ } finally {
+ thriftServer.stop();
+ }
+ }
+
+ /**
+ * See if random port choosing works around protocol port clashes
+ */
+ @Test
+ public void testInfoPortClash() {
+ ThriftServer thriftServer = null;
+ try {
+ thriftServer = new TestThriftServerCmdLine(null, false, false, false).
+ createBoundServer(false, true);
assertNotNull(thriftServer.tserver);
} finally {
thriftServer.stop();
diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java
index 159c359..351f744 100644
--- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java
+++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java
@@ -49,7 +49,6 @@ import org.apache.thrift.server.TServer;
import org.apache.thrift.transport.TFramedTransport;
import org.apache.thrift.transport.TSocket;
import org.apache.thrift.transport.TTransport;
-import org.apache.thrift.transport.TTransportException;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
@@ -176,16 +175,40 @@ public class TestThriftServerCmdLine {
* Server can fail to bind if clashing address. Add retrying until we get a good server.
*/
ThriftServer createBoundServer() {
- return createBoundServer(false);
+ return createBoundServer(false, false);
+ }
+
+ private ServerSocket getBoundSocket() {
+ ServerSocket ss = null;
+ while (true) {
+ port = getRandomPort();
+ try {
+ ss = new ServerSocket();
+ ss.bind(new InetSocketAddress(port));
+ break;
+ } catch (IOException ioe) {
+ LOG.warn("Failed bind", ioe);
+ try {
+ ss.close();
+ } catch (IOException ioe2) {
+ LOG.warn("FAILED CLOSE of failed bind socket", ioe2);
+ }
+ }
+ }
+ return ss;
}
/**
- * @param clash This param is just so we can manufacture a port clash so we can test the
- * code does the right thing when this happens during actual test runs. Ugly but works.
- * @see TestBindExceptionHandling#testPortClash()
+ * @param protocolPortClash This param is just so we can manufacture a port clash so we can test
+ * the code does the right thing when this happens during actual test runs. Ugly but works.
+ * @see TestBindExceptionHandling#testProtocolPortClash()
*/
- ThriftServer createBoundServer(boolean clash) {
- boolean testClashOfFirstPort = clash;
+ ThriftServer createBoundServer(boolean protocolPortClash, boolean infoPortClash) {
+ if (protocolPortClash && infoPortClash) {
+ throw new RuntimeException("Can't set both at same time");
+ }
+ boolean testClashOfFirstProtocolPort = protocolPortClash;
+ boolean testClashOfFirstInfoPort = infoPortClash;
List<String> args = new ArrayList<>();
ServerSocket ss = null;
for (int i = 0; i < 100; i++) {
@@ -195,22 +218,26 @@ public class TestThriftServerCmdLine {
assertTrue(serverTypeOption.startsWith("-"));
args.add(serverTypeOption);
}
- port = getRandomPort();
- if (testClashOfFirstPort) {
+ if (testClashOfFirstProtocolPort) {
// Test what happens if already something bound to the socket.
// Occupy the random port we just pulled.
- try {
- ss = new ServerSocket();
- ss.bind(new InetSocketAddress(port));
- } catch (IOException ioe) {
- LOG.warn("Failed bind", ioe);
- }
- testClashOfFirstPort = false;
+ ss = getBoundSocket();
+ port = ss.getLocalPort();
+ testClashOfFirstProtocolPort = false;
+ } else {
+ port = getRandomPort();
}
args.add("-" + PORT_OPTION);
args.add(String.valueOf(port));
args.add("-" + INFOPORT_OPTION);
- int infoPort = getRandomPort();
+ int infoPort;
+ if (testClashOfFirstInfoPort) {
+ ss = getBoundSocket();
+ infoPort = ss.getLocalPort();
+ testClashOfFirstInfoPort = false;
+ } else {
+ infoPort = getRandomPort();
+ }
args.add(String.valueOf(infoPort));
if (specifyFramed) {
@@ -231,9 +258,8 @@ public class TestThriftServerCmdLine {
for (int ii = 0; ii < 100 && (thriftServer.tserver == null); ii++) {
Threads.sleep(100);
}
- if (cmdLineException instanceof TTransportException &&
- cmdLineException.getCause() instanceof BindException) {
- LOG.info("Trying new port", cmdLineException);
+ if (isBindException(cmdLineException)) {
+ LOG.info("BindException; trying new port", cmdLineException);
cmdLineException = null;
thriftServer.stop();
continue;
@@ -254,6 +280,20 @@ public class TestThriftServerCmdLine {
return thriftServer;
}
+ private boolean isBindException(Exception cmdLineException) {
+ if (cmdLineException == null) {
+ return false;
+ }
+ if (cmdLineException instanceof BindException) {
+ return true;
+ }
+ if (cmdLineException.getCause() != null &&
+ cmdLineException.getCause() instanceof BindException) {
+ return true;
+ }
+ return false;
+ }
+
@Test
public void testRunThriftServer() throws Exception {
ThriftServer thriftServer = createBoundServer();