You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "tkhurana (via GitHub)" <gi...@apache.org> on 2023/07/21 19:02:28 UTC
[GitHub] [phoenix] tkhurana opened a new pull request, #1643: PHOENIX-6995 HA client connections ignore additional jdbc params in t…
tkhurana opened a new pull request, #1643:
URL: https://github.com/apache/phoenix/pull/1643
…he jdbc string
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on a diff in pull request #1643: PHOENIX-6995 HA client connections ignore additional jdbc params in t…
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #1643:
URL: https://github.com/apache/phoenix/pull/1643#discussion_r1271568394
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java:
##########
@@ -770,6 +786,28 @@ public String getUrl2() {
return urls.getSecond();
}
+ public String getJDBCUrl(String zkUrl) {
+ Preconditions.checkArgument(zkUrl.equals(getUrl1()) || zkUrl.equals(getUrl2()),
+ "The URL '" + zkUrl + "' does not belong to this HA group " + this);
+ StringBuilder sb = new StringBuilder();
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL);
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR);
+ sb.append(zkUrl);
+ if (!Strings.isNullOrEmpty(additionalJDBCParams)) {
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR);
+ sb.append(additionalJDBCParams);
+ }
+ return sb.toString();
+ }
+
+ public String getJDBCUrl1() {
+ return getJDBCUrl(urls.getFirst());
Review Comment:
nit: `getJDBCUrl(getUrl1())`
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java:
##########
@@ -770,6 +786,28 @@ public String getUrl2() {
return urls.getSecond();
}
+ public String getJDBCUrl(String zkUrl) {
+ Preconditions.checkArgument(zkUrl.equals(getUrl1()) || zkUrl.equals(getUrl2()),
+ "The URL '" + zkUrl + "' does not belong to this HA group " + this);
+ StringBuilder sb = new StringBuilder();
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL);
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR);
+ sb.append(zkUrl);
+ if (!Strings.isNullOrEmpty(additionalJDBCParams)) {
+ sb.append(PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR);
+ sb.append(additionalJDBCParams);
+ }
+ return sb.toString();
+ }
+
+ public String getJDBCUrl1() {
+ return getJDBCUrl(urls.getFirst());
+ }
+
+ public String getJDBCUrl2() {
+ return getJDBCUrl(urls.getSecond());
Review Comment:
nit: `getJDBCUrl(getUrl2())`
##########
phoenix-core/src/main/java/org/apache/phoenix/util/JDBCUtil.java:
##########
@@ -219,7 +221,10 @@ public static String formatZookeeperUrl(String zkUrl){
Preconditions.checkArgument(hosts.length > 0,"Unexpected zk url format no hosts found.");
String hostsStrings = Arrays.stream(hosts).sorted().collect(Collectors.joining(","));
components[0] = hostsStrings;
- return Arrays.stream(components).collect(Collectors.joining(":"));
+ // host:port:path:principal
+ // additional arguments passed in url, strip them out
+ int endIdx = Integer.min(components.length, 3);
Review Comment:
nit: i wonder if we should also make initial validation check for len: 3 or 4
```
Preconditions.checkArgument(components.length == 3 || components.length == 4, "Unexpected zk url format.");
```
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana merged pull request #1643: PHOENIX-6995 HA client connections ignore additional jdbc params in t…
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana merged PR #1643:
URL: https://github.com/apache/phoenix/pull/1643
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1643: PHOENIX-6995 HA client connections ignore additional jdbc params in t…
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1643:
URL: https://github.com/apache/phoenix/pull/1643#discussion_r1272548442
##########
phoenix-core/src/main/java/org/apache/phoenix/util/JDBCUtil.java:
##########
@@ -219,7 +221,10 @@ public static String formatZookeeperUrl(String zkUrl){
Preconditions.checkArgument(hosts.length > 0,"Unexpected zk url format no hosts found.");
String hostsStrings = Arrays.stream(hosts).sorted().collect(Collectors.joining(","));
components[0] = hostsStrings;
- return Arrays.stream(components).collect(Collectors.joining(":"));
+ // host:port:path:principal
+ // additional arguments passed in url, strip them out
+ int endIdx = Integer.min(components.length, 3);
Review Comment:
Actually, the number of components can be greater than 4 also
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org