[
https://issues.apache.org/jira/browse/PHOENIX-6523?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17407410#comment-17407410
]
ASF GitHub Bot commented on PHOENIX-6523:
-----------------------------------------
stoty commented on a change in pull request #1280:
URL: https://github.com/apache/phoenix/pull/1280#discussion_r698988086
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -33,6 +33,10 @@
import javax.annotation.concurrent.Immutable;
+import org.apache.phoenix.jdbc.bootstrapz.HBaseRegistryBootstrap;
Review comment:
can we have just org.apache.phoenix.jdbc.bootstrap. ?
We generally don't use plurals in package names
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java
##########
@@ -19,6 +19,7 @@
import static
org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkNotNull;
import static
org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkArgument;
+import static
org.apache.phoenix.thirdparty.com.google.common.collect.Sets.newHashSet;
Review comment:
seems to be unused imports.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -199,15 +201,20 @@ public void close() throws SQLException {
private static final Object KERBEROS_LOGIN_LOCK = new Object();
private static final char WINDOWS_SEPARATOR_CHAR = '\\';
private static final String REALM_EQUIVALENCY_WARNING_MSG = "Provided
principal does not contan a realm and the default realm cannot be determined.
Ignoring realm equivalency check.";
+
private static SQLException getMalFormedUrlException(String url) {
return new
SQLExceptionInfo.Builder(SQLExceptionCode.MALFORMED_CONNECTION_URL)
.setMessage(url).build().buildException();
}
-
- public String getZookeeperConnectionString() {
- return getZookeeperQuorum() + ":" + getPort();
- }
-
+
+ private static SQLException getMalFormedUrlException(String message,
String url) {
+ return getMalFormedUrlException(message + "\n" + url);
+ }
+
+ public String getZookeeperConnectionString() {
Review comment:
This is used by Tephra only, which won't be able to make sense of a HRpc
quorum.
Better add a check here, and fail early if the boostrap is not ZK.
(or maybe figure out the cluster ZK quorum somehow, it is possible)
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/bootstrapz/HRpcHBaseRegistryBootstrap.java
##########
@@ -0,0 +1,44 @@
+package org.apache.phoenix.jdbc.bootstrapz;
Review comment:
I think that moving parameter parsing to the bootstrap module, and
having an API like:
```
BootstrapRegistry bs = BootstrapFactory.fromURL(String URL);
Map connpProps = bs.generateConnectionProps();
bs.toString()
bs.getZookeperQuorum() (which fails for HRpc)
bs.equals(), bs.hashMap()
```
would be a better encapsulation, and simplify code.
WDYT @joshelser (as this was originally your suggestion) ?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -604,12 +645,15 @@ public boolean equals(Object obj) {
if (keytab == null) {
if (other.keytab != null) return false;
} else if (!keytab.equals(other.keytab)) return false;
+ if (bootstrap == null) {
+ if (other.bootstrap != null) return false;
+ } else if (!bootstrap.equals(other.bootstrap)) return false;
return true;
}
@Override
public String toString() {
- return zookeeperQuorum + (port == null ? "" : ":" +
port)
+ return quorum + (port == null ? "" : ":" + port)
Review comment:
This loses the bootstrap type info.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -225,16 +232,45 @@ private static boolean isMultiPortUrl(String portStr) {
}
return false;
}
-
+
public static ConnectionInfo create(String url) throws SQLException {
url = url == null ? "" : url;
if (url.isEmpty() || url.equalsIgnoreCase("jdbc:phoenix:")
|| url.equalsIgnoreCase("jdbc:phoenix")) {
return defaultConnectionInfo(url);
}
+
+ // ex: jdbc:phoenix+hrpc:hostname1....
url = url.startsWith(PhoenixRuntime.JDBC_PROTOCOL)
? url.substring(PhoenixRuntime.JDBC_PROTOCOL.length())
: PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR + url;
+
+ HBaseRegistryBootstrapType bootstrapAsEnum = null;
+
+ // ex: +hrpc:hostname1....
+ if
(url.startsWith(String.valueOf(PhoenixRuntime.JDBC_PROTOCOL_CONNECTOR_PREFIX)))
{
+ int offset =
url.indexOf(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR);
+ final String firstToken = url.substring(0, offset);
+
+ // ex: +hrpc
+ final String bootstrapString = firstToken
+
.replace(String.valueOf(PhoenixRuntime.JDBC_PROTOCOL_CONNECTOR_PREFIX), "")
+ .toUpperCase();
+
+ if (Strings.isNullOrEmpty(bootstrapString)) {
+ throw getMalFormedUrlException(url);
Review comment:
IUC this will not allow URLs without bootstraps specified.
We should support bootstrap-less URLs, and default to ZK for backwards
compatibility.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -199,15 +201,20 @@ public void close() throws SQLException {
private static final Object KERBEROS_LOGIN_LOCK = new Object();
private static final char WINDOWS_SEPARATOR_CHAR = '\\';
private static final String REALM_EQUIVALENCY_WARNING_MSG = "Provided
principal does not contan a realm and the default realm cannot be determined.
Ignoring realm equivalency check.";
+
private static SQLException getMalFormedUrlException(String url) {
return new
SQLExceptionInfo.Builder(SQLExceptionCode.MALFORMED_CONNECTION_URL)
.setMessage(url).build().buildException();
}
-
- public String getZookeeperConnectionString() {
- return getZookeeperQuorum() + ":" + getPort();
- }
-
+
+ private static SQLException getMalFormedUrlException(String message,
String url) {
+ return getMalFormedUrlException(message + "\n" + url);
+ }
+
+ public String getZookeeperConnectionString() {
Review comment:
This is used by Tephra only, which won't be able to make sense of a HRpc
quorum.
Better add a check here, and fail early if the boostrap is not ZK.
(or maybe figure out the cluster ZK quorum somehow, if it is possible)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Support for HBase Registry Implementations through Phoenix connection URL
> -------------------------------------------------------------------------
>
> Key: PHOENIX-6523
> URL: https://issues.apache.org/jira/browse/PHOENIX-6523
> Project: Phoenix
> Issue Type: Improvement
> Components: core
> Reporter: Ramie Raufdeen
> Priority: Major
>
> https://issues.apache.org/jira/browse/HBASE-23305
> https://issues.apache.org/jira/browse/HBASE-18095
>
> HBase now supports a zookeeper-less connection strategy using a Master
> Registry implementation.
>
> For this to work, the client simply needs to set a list of <host:port>s of
> the HMaster quorum
>
> {code:java}
> <property>
> <name>hbase.masters</name>
> <value>master1:16000,master2:16001,master3:16000</value>
> </property>
> {code}
>
> To support opting into this from a Phoenix connection URL, we can introduce a
> "connector type". We'll leverage the *+* char of [JDBC URL
> grammar|https://docs.oracle.com/cd/E17952_01/connector-j-8.0-en/connector-j-reference-jdbc-url-format.html]
> to specify the connection type. Connections will start to look something
> like this:
> {code:java}
> jdbc:phoenix+zk:hostname1,2,3...:<properties>
> jdbc:phoenix+hrpc:hostname1,2,3...:<properties>
> jdbc:phoenix+bigtable:hostname1,2,3...:<properties>{code}
> Above are examples of opting into hrpc/zk/bigtable registry implementations
> of HBase.
>
> If no connector is specified, the driver will default to a Zookeeper based
> connection.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
