You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by jy...@apache.org on 2014/09/04 18:44:20 UTC

git commit: PHOENIX-1234 QueryUtil doesn't parse zk hosts correctly

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 <jy...@apache.org>
Authored: Thu Sep 4 09:44:10 2014 -0700
Committer: Jesse Yates <jy...@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