From a675f17778e75638abe92b02c56ee1791b408e4b Mon Sep 17 00:00:00 2001 From: stack Date: Wed, 1 Apr 2020 12:23:09 -0700 Subject: [PATCH] HBASE-24100 [Flakey Tests] Add test to check we work properly when port clash setting up thriftserver --- .../thrift/TestBindExceptionHandling.java | 42 +++++++++++++++++++ .../hbase/thrift/TestThriftServerCmdLine.java | 42 ++++++++++++++++++- .../thrift2/TestThrift2ServerCmdLine.java | 1 - 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java 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 new file mode 100644 index 0000000000..0658ed80f3 --- /dev/null +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestBindExceptionHandling.java @@ -0,0 +1,42 @@ +/* + * 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.hbase.thrift; + +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import static org.junit.Assert.assertNotNull; + +@Category({ ClientTests.class, MediumTests.class}) +public class TestBindExceptionHandling { + /** + * See if random port choosing works around port clashes + */ + @Test + public void testPortClash() { + ThriftServer thriftServer = null; + try { + thriftServer = new TestThriftServerCmdLine(null, false, false, false). + createBoundServer(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 45e36d580b..1aa0a5c4d3 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 @@ -24,8 +24,11 @@ import static org.apache.hadoop.hbase.thrift.Constants.INFOPORT_OPTION; import static org.apache.hadoop.hbase.thrift.Constants.PORT_OPTION; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.io.IOException; import java.net.BindException; import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -164,22 +167,49 @@ public class TestThriftServerCmdLine { return new ThriftServer(TEST_UTIL.getConfiguration()); } + private int getRandomPort() { + return HBaseTestingUtility.randomFreePort(); + } + /** * Server can fail to bind if clashing address. Add retrying until we get a good server. */ ThriftServer createBoundServer() { + return createBoundServer(false); + } + + /** + * @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() + */ + ThriftServer createBoundServer(boolean clash) { + boolean testClashOfFirstPort = clash; List args = new ArrayList<>(); + ServerSocket ss = null; for (int i = 0; i < 100; i++) { + args.clear(); if (implType != null) { String serverTypeOption = implType.toString(); assertTrue(serverTypeOption.startsWith("-")); args.add(serverTypeOption); } - port = HBaseTestingUtility.randomFreePort(); + port = getRandomPort(); + if (testClashOfFirstPort) { + // 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; + } args.add("-" + PORT_OPTION); args.add(String.valueOf(port)); args.add("-" + INFOPORT_OPTION); - int infoPort = HBaseTestingUtility.randomFreePort(); + int infoPort = getRandomPort(); args.add(String.valueOf(infoPort)); if (specifyFramed) { @@ -203,11 +233,19 @@ public class TestThriftServerCmdLine { if (cmdLineException instanceof TTransportException && cmdLineException.getCause() instanceof BindException) { LOG.info("Trying new port", cmdLineException); + cmdLineException = null; thriftServer.stop(); continue; } break; } + if (ss != null) { + try { + ss.close(); + } catch (IOException ioe) { + LOG.warn("Failed close", ioe); + } + } Class expectedClass = implType != null ? implType.serverClass : TBoundedThreadPoolServer.class; assertEquals(expectedClass, thriftServer.tserver.getClass()); diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThrift2ServerCmdLine.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThrift2ServerCmdLine.java index 2f719b925f..1b8ed2d897 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThrift2ServerCmdLine.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThrift2ServerCmdLine.java @@ -94,5 +94,4 @@ public class TestThrift2ServerCmdLine extends TestThriftServerCmdLine { sock.close(); } } - } -- 2.19.1