Repository: phoenix Updated Branches: refs/heads/master ec3be54ef -> 4a1ec7ec4
PHOENIX-1234 QueryUtil doesn't parse zk hosts correctly Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/4a1ec7ec Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/4a1ec7ec Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/4a1ec7ec Branch: refs/heads/master Commit: 4a1ec7ec44248315023db41cf4c941a366a1d294 Parents: ec3be54 Author: Jesse Yates <jya...@apache.org> Authored: Thu Sep 4 09:44:10 2014 -0700 Committer: Jesse Yates <jya...@apache.org> Committed: Thu Sep 4 09:44:10 2014 -0700 ---------------------------------------------------------------------- .../java/org/apache/phoenix/util/QueryUtil.java | 55 +++++++++++++++----- .../java/org/apache/phoenix/query/BaseTest.java | 16 +++--- .../org/apache/phoenix/util/QueryUtilTest.java | 55 +++++++++++++++++++- 3 files changed, 102 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/4a1ec7ec/phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java index 6a45666..88ffd8e 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java @@ -25,6 +25,7 @@ import java.sql.DatabaseMetaData; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.ArrayList; import java.util.List; import java.util.Properties; @@ -35,6 +36,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.util.Addressing; +import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.zookeeper.ZKConfig; import org.apache.phoenix.jdbc.PhoenixDriver; @@ -43,6 +45,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import org.apache.phoenix.query.QueryServices; public final class QueryUtil { @@ -189,26 +192,50 @@ public final class QueryUtil { } public static Connection getConnection(Properties props, Configuration conf) + throws ClassNotFoundException, + SQLException { + String url = getConnectionUrl(props, conf); + LOG.info("Creating connection with the jdbc url:" + url); + return DriverManager.getConnection(url, props); + } + + public static String getConnectionUrl(Properties props, Configuration conf) throws ClassNotFoundException, SQLException { // make sure we load the phoenix driver Class.forName(PhoenixDriver.class.getName()); // read the hbase properties from the configuration String server = ZKConfig.getZKQuorumServersString(conf); - int port; - // if it has a port, don't try to add one - try { - server = Addressing.parseHostname(server); - port = Addressing.parsePort(server); - } catch (IllegalArgumentException e) { - // port isn't set - port = - conf.getInt(HConstants.ZOOKEEPER_CLIENT_PORT, - HConstants.DEFAULT_ZOOKEPER_CLIENT_PORT); + // could be a comma-separated list + String[] rawServers = server.split(","); + List<String> servers = new ArrayList<String>(rawServers.length); + boolean first = true; + int port = -1; + for (String serverPort : rawServers) { + try { + server = Addressing.parseHostname(serverPort); + int specifiedPort = Addressing.parsePort(serverPort); + // there was a previously specified port and it doesn't match this server + if (port > 0 && specifiedPort != port) { + throw new IllegalStateException("Phoenix/HBase only supports connecting to a " + + "single zookeeper client port. Specify servers only as host names in " + + "HBase configuration"); + } + // set the port to the specified port + port = specifiedPort; + servers.add(server); + } catch (IllegalArgumentException e) { + } + } + // port wasn't set, shouldn't ever happen from HBase, but just in case + if (port == -1) { + port = conf.getInt(QueryServices.ZOOKEEPER_PORT_ATTRIB, -1); + if (port == -1) { + throw new RuntimeException("Client zk port was not set!"); + } } + server = Joiner.on(',').join(servers); - String jdbcUrl = getUrl(server, port); - LOG.info("Creating connection with the jdbc url:" + jdbcUrl); - return DriverManager.getConnection(jdbcUrl, props); + return getUrl(server, port); } -} +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/phoenix/blob/4a1ec7ec/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java index c57b555..e17e9bf 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java @@ -125,11 +125,7 @@ import org.apache.phoenix.schema.NewerTableAlreadyExistsException; import org.apache.phoenix.schema.PTableType; import org.apache.phoenix.schema.TableAlreadyExistsException; import org.apache.phoenix.schema.TableNotFoundException; -import org.apache.phoenix.util.ConfigUtil; -import org.apache.phoenix.util.PhoenixRuntime; -import org.apache.phoenix.util.PropertiesUtil; -import org.apache.phoenix.util.ReadOnlyProps; -import org.apache.phoenix.util.SchemaUtil; +import org.apache.phoenix.util.*; import org.junit.Assert; import com.google.common.collect.ImmutableMap; @@ -476,8 +472,6 @@ public abstract class BaseTest { utility = new HBaseTestingUtility(conf); try { utility.startMiniCluster(); - String clientPort = utility.getConfiguration().get(QueryServices.ZOOKEEPER_PORT_ATTRIB); - // add shutdown hook to kill the mini cluster Runtime.getRuntime().addShutdownHook(new Thread() { @Override @@ -489,12 +483,16 @@ public abstract class BaseTest { } } }); - return JDBC_PROTOCOL + JDBC_PROTOCOL_SEPARATOR + LOCALHOST + JDBC_PROTOCOL_SEPARATOR + clientPort - + JDBC_PROTOCOL_TERMINATOR + PHOENIX_TEST_DRIVER_URL_PARAM; + return getLocalClusterUrl(utility); } catch (Throwable t) { throw new RuntimeException(t); } } + + protected static String getLocalClusterUrl(HBaseTestingUtility util) throws Exception { + String url = QueryUtil.getConnectionUrl(new Properties(), util.getConfiguration()); + return url + PHOENIX_TEST_DRIVER_URL_PARAM; + } /** * Initialize the cluster in distributed mode http://git-wip-us.apache.org/repos/asf/phoenix/blob/4a1ec7ec/phoenix-core/src/test/java/org/apache/phoenix/util/QueryUtilTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/QueryUtilTest.java b/phoenix-core/src/test/java/org/apache/phoenix/util/QueryUtilTest.java index 0ac2bbc..48929ed 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/util/QueryUtilTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/util/QueryUtilTest.java @@ -18,13 +18,21 @@ package org.apache.phoenix.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import java.sql.Connection; import java.sql.Types; +import java.util.Properties; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; import org.junit.Test; import com.google.common.collect.ImmutableList; +import javax.management.Query; + public class QueryUtilTest { private static final ColumnInfo ID_COLUMN = new ColumnInfo("ID", Types.BIGINT); @@ -61,4 +69,49 @@ public class QueryUtilTest { "SELECT \"ID\",\"NAME\" FROM \"MYTAB\"", QueryUtil.constructSelectStatement("MYTAB", ImmutableList.of(ID_COLUMN,NAME_COLUMN))); } -} + + /** + * Test that we create connection strings from the HBase Configuration that match the + * expected syntax. Expected to log exceptions as it uses ZK host names that don't exist + * @throws Exception on failure + */ + @Test + public void testCreateConnectionFromConfiguration() throws Exception { + Properties props = new Properties(); + // standard lookup. this already checks if we set hbase.zookeeper.clientPort + Configuration conf = new Configuration(false); + conf.set(HConstants.ZOOKEEPER_QUORUM, "localhost"); + conf.set(HConstants.ZOOKEEPER_CLIENT_PORT, "2181"); + String conn = QueryUtil.getConnectionUrl(props, conf); + validateUrl(conn); + + // set the zks to a few hosts, some of which are no online + conf.set(HConstants.ZOOKEEPER_QUORUM, "host.at.some.domain.1,localhost," + + "host.at.other.domain.3"); + conn = QueryUtil.getConnectionUrl(props, conf); + validateUrl(conn); + + // and try with different leader/peer ports + conf.set("hbase.zookeeper.peerport", "3338"); + conf.set("hbase.zookeeper.leaderport", "3339"); + conn = QueryUtil.getConnectionUrl(props, conf); + validateUrl(conn); + } + + private void validateUrl(String url) { + String prefix = QueryUtil.getUrl(""); + assertTrue("JDBC URL missing jdbc protocol prefix", url.startsWith(prefix)); + //remove the prefix, should only be left with server,server...:port + url = url.substring(prefix.length()+1); + // make sure only a single ':' + assertEquals("More than a single ':' in url: "+url, url.indexOf(PhoenixRuntime + .JDBC_PROTOCOL_SEPARATOR), + url.lastIndexOf(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR)); + // make sure that each server is comma separated + url = url.substring(0, url.indexOf(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR)); + String[] servers = url.split(","); + for(String server: servers){ + assertFalse("Found whitespace in server names for url: " + url, server.contains(" ")); + } + } +} \ No newline at end of file