This is an automated email from the ASF dual-hosted git repository.
tkhurana pushed a commit to branch PHOENIX-7562-feature-new
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/PHOENIX-7562-feature-new by
this push:
new 95bc49ae23 PHOENIX-7763 :- Accept HAGroupName in Urls for HA
Connection to improve backward compatibility. (#2378)
95bc49ae23 is described below
commit 95bc49ae23ff31efae8b76f47931272dc149c467
Author: Lokesh Khurana <[email protected]>
AuthorDate: Thu Feb 26 15:39:27 2026 -0800
PHOENIX-7763 :- Accept HAGroupName in Urls for HA Connection to improve
backward compatibility. (#2378)
* Overriding connection properties with URL passed properties in HA
Connections
---
.../apache/phoenix/jdbc/HighAvailabilityGroup.java | 50 +++++---
.../apache/phoenix/jdbc/PhoenixEmbeddedDriver.java | 2 +-
.../phoenix/jdbc/FailoverPhoenixConnectionIT.java | 28 ++++-
.../jdbc/HighAvailabilityTestingUtility.java | 1 +
.../phoenix/jdbc/HighAvailabilityGroupTest.java | 126 +++++++++++++++++++++
5 files changed, 185 insertions(+), 22 deletions(-)
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java
index 87a6700eca..77c7e75c13 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/HighAvailabilityGroup.java
@@ -240,13 +240,6 @@ public class HighAvailabilityGroup {
* @throws SQLException if fails to get HA information and/or invalid
properties are seen
*/
public static HAURLInfo getUrlInfo(String url, Properties properties) throws
SQLException {
- // Check if HA group name is provided in the properties if not throw an
exception
- String name = properties.getProperty(PHOENIX_HA_GROUP_ATTR);
- if (StringUtils.isEmpty(name)) {
- throw new
SQLExceptionInfo.Builder(SQLExceptionCode.HA_INVALID_PROPERTIES)
- .setMessage(String.format("HA group name can not be empty for HA URL
%s", url)).build()
- .buildException();
- }
url = checkUrl(url);
String principal = null;
String additionalJDBCParams = null;
@@ -303,23 +296,45 @@ public class HighAvailabilityGroup {
: additionalJDBCParams)
: null;
- HAURLInfo haurlInfo = new HAURLInfo(name, principal, additionalJDBCParams);
- HAGroupInfo info = getHAGroupInfo(url, properties);
- URLS.computeIfAbsent(info, haGroupInfo -> new HashSet<>()).add(haurlInfo);
- return haurlInfo;
- }
+ // Here additionalJDBCParams can contain more than one properties
separated by ;
+ // Adding the properties from url to connection properties
+ if (additionalJDBCParams != null) {
+ String[] propsFromUrl = additionalJDBCParams.split(";");
+ for (String property : propsFromUrl) {
+ if (StringUtils.isEmpty(property)) {
+ continue;
+ }
+ String[] keyValue = property.split("=");
+ if (keyValue.length != 2) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.MALFORMED_CONNECTION_URL)
+ .setMessage(String.format("URL %s has additional JDBC props in
wrong format", url))
+ .build().buildException();
+ }
+ properties.put(keyValue[0], keyValue[1]);
+ }
+ }
- private static HAGroupInfo getHAGroupInfo(String url, Properties properties)
throws SQLException {
+ // Check if HA group name is provided in the url if not then check in the
properties if not
+ // throw an exception.
String name = properties.getProperty(PHOENIX_HA_GROUP_ATTR);
if (StringUtils.isEmpty(name)) {
throw new
SQLExceptionInfo.Builder(SQLExceptionCode.HA_INVALID_PROPERTIES)
.setMessage(String.format("HA group name can not be empty for HA URL
%s", url)).build()
.buildException();
}
+
+ HAURLInfo haurlInfo = new HAURLInfo(name, principal, additionalJDBCParams);
+ HAGroupInfo info = getHAGroupInfo(url, properties, haurlInfo);
+ URLS.computeIfAbsent(info, haGroupInfo -> new HashSet<>()).add(haurlInfo);
+ return haurlInfo;
+ }
+
+ private static HAGroupInfo getHAGroupInfo(String url, Properties properties,
HAURLInfo haurlInfo)
+ throws SQLException {
url = checkUrl(url);
url = url.substring(url.indexOf("[") + 1, url.indexOf("]"));
String[] urls = url.split("\\|");
- return new HAGroupInfo(name, urls[0], urls[1]);
+ return new HAGroupInfo(haurlInfo.getName(), urls[0], urls[1]);
}
/**
@@ -371,7 +386,8 @@ public class HighAvailabilityGroup {
*/
public static Optional<HighAvailabilityGroup> get(String url, Properties
properties)
throws SQLException {
- HAGroupInfo info = getHAGroupInfo(url, properties);
+ HAURLInfo haurlInfo = getUrlInfo(url, properties);
+ HAGroupInfo info = getHAGroupInfo(url, properties, haurlInfo);
// create a cache for missing CRR to prevent unnecessary exceptions (cache
expires in 5 minutes)
if (MISSING_CRR_GROUPS_CACHE.getIfPresent(info) != null) {
return Optional.empty();
@@ -419,9 +435,9 @@ public class HighAvailabilityGroup {
* protocol
* @throws SQLException if fails to get HA information and/or invalid
properties are seen
*/
- static Optional<String> getFallbackCluster(String url, Properties properties)
+ static Optional<String> getFallbackCluster(String url, Properties
properties, HAURLInfo haurlInfo)
throws SQLException {
- HAGroupInfo haGroupInfo = getHAGroupInfo(url, properties);
+ HAGroupInfo haGroupInfo = getHAGroupInfo(url, properties, haurlInfo);
String fallback =
properties.getProperty(PHOENIX_HA_SHOULD_FALLBACK_WHEN_MISSING_CRR_KEY,
PHOENIX_HA_SHOULD_FALLBACK_WHEN_MISSING_CRR_DEFAULT);
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
index b5bbe2ea55..5670bc3c33 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
@@ -138,7 +138,7 @@ public abstract class PhoenixEmbeddedDriver implements
Driver, SQLCloseable {
return haGroup.get().connect(augmentedInfo, haurlInfo);
} else {
// If empty HA group is returned, fall back to single cluster.
- url = HighAvailabilityGroup.getFallbackCluster(url, info).orElseThrow(
+ url = HighAvailabilityGroup.getFallbackCluster(url, info,
haurlInfo).orElseThrow(
() -> new SQLException("HA group can not be initialized, fallback to
single cluster"));
}
}
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java
index f74b91a57e..bc86689077 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/FailoverPhoenixConnectionIT.java
@@ -62,6 +62,7 @@ import org.apache.phoenix.jdbc.ClusterRoleRecord.ClusterRole;
import org.apache.phoenix.monitoring.MetricType;
import org.apache.phoenix.query.ConnectionQueryServices;
import org.apache.phoenix.query.ConnectionQueryServicesImpl;
+import org.apache.phoenix.schema.TypeMismatchException;
import org.apache.phoenix.util.PhoenixRuntime;
import org.junit.After;
import org.junit.AfterClass;
@@ -579,7 +580,7 @@ public class FailoverPhoenixConnectionIT extends HABaseIT {
tableName = tableName + "Tenant";
CLUSTERS.createTenantSpecificTable(haGroup, tableName);
- clientProperties.setProperty("TenantId", "mytenant");
+ clientProperties.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, "mytenant");
Connection tenantConn = createFailoverConnection();
doTestBasicOperationsWithConnection(tenantConn, tableName, haGroupName);
@@ -593,6 +594,24 @@ public class FailoverPhoenixConnectionIT extends HABaseIT {
try (Connection newTenantConn = createFailoverConnection()) {
doTestBasicOperationsWithConnection(newTenantConn, tableName,
haGroupName);
}
+
+ // Test creating new tenant connection with url with tenant id
+ clientProperties.remove(PhoenixRuntime.TENANT_ID_ATTRIB);
+ String jdbcUrl =
+ CLUSTERS.getJdbcHAUrl() + ":" + PhoenixRuntime.TENANT_ID_ATTRIB + "=" +
"mytenant";
+ Connection tenantConn2 = DriverManager.getConnection(jdbcUrl,
clientProperties);
+ doTestBasicOperationsWithConnection(tenantConn2, tableName, haGroupName);
+
+ // Normal Connection should fail
+ Connection conn3 = createFailoverConnection();
+ try {
+ doTestBasicOperationsWithConnection(conn3, tableName, haGroupName);
+ fail(
+ "Should have failed since normal connection should not be able to
connect to tenant specific table");
+ } catch (TypeMismatchException e) {
+ LOG.info("Got expected exception when creating normal connection to
tenant specific table",
+ e);
+ }
}
/**
@@ -809,9 +828,10 @@ public class FailoverPhoenixConnectionIT extends HABaseIT {
}
// For the given ha group of current test the value in URLS set for
current haGroupInfo
- // should be numThreads + 2 as all the connections created with same
principal should have
- // one entry in map and one extra for null principal
- Assert.assertEquals(numThreads + 2,
URLS.get(haGroup.getGroupInfo()).size());
+ // should be numThreads + 3 as all the connections created with same
principal should have
+ // one entry in map and one extra for null principal and one from the
setup() method for
+ // USER_FOO principal
+ Assert.assertEquals(numThreads + 3,
URLS.get(haGroup.getGroupInfo()).size());
}
/**
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java
index cf30d17ca8..4aa509ffdd 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/jdbc/HighAvailabilityTestingUtility.java
@@ -1155,6 +1155,7 @@ public class HighAvailabilityTestingUtility {
}
public static void sleepThreadFor(long sleepTime) {
+ LOG.info("Sleeping thread for {} milliseconds", sleepTime);
long start = System.currentTimeMillis();
while (System.currentTimeMillis() - start < sleepTime) {
try {
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/jdbc/HighAvailabilityGroupTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/HighAvailabilityGroupTest.java
new file mode 100644
index 0000000000..8f52b5110c
--- /dev/null
+++
b/phoenix-core/src/test/java/org/apache/phoenix/jdbc/HighAvailabilityGroupTest.java
@@ -0,0 +1,126 @@
+/*
+ * 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.phoenix.jdbc;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class HighAvailabilityGroupTest {
+ private static final Logger LOG =
LoggerFactory.getLogger(FailoverPhoenixConnectionTest.class);
+ String quorum1 = "master1\\\\:60010,master2\\\\:60000,master3\\\\:60010";
+ String quorum2 =
"peer_master1\\\\:60010,peer_master2\\\\:60000,peer_master3\\\\:60010";
+
+ @Test
+ public void testGetUrlInfo() throws Exception {
+ String correctAdditionalParams =
"phoenix.ha.group.name=testGetUrlInfo;key2=value2;key3=value3";
+ String correctAdditionalParams2 =
+ "key1=val1;phoenix.ha.group.name=testGetUrlInfo;key2=value2;key3=value3";
+ String correctAdditionalParams3 =
"key1=val1;key2=val2;phoenix.ha.group.name=testGetUrlInfo";
+ String incorrectAdditionalParams = "key1;key2=val2";
+ String missingAdditionalParams = "key1=value1";
+ String principal = "principal";
+ String urlFormatWithPrincipal = "jdbc:phoenix+rpc:[%s|%s]:%s:%s";
+ String urlFormatWithoutPrincipal = "jdbc:phoenix+rpc:[%s|%s]::%s";
+
+ // Test correct additional params with principal
+ String url =
+ String.format(urlFormatWithPrincipal, quorum1, quorum2, principal,
correctAdditionalParams);
+ getAndAssertUrlInfo(url, correctAdditionalParams, principal);
+
+ // Test correct additional params without principal
+ url = String.format(urlFormatWithoutPrincipal, quorum1, quorum2,
correctAdditionalParams);
+ getAndAssertUrlInfo(url, correctAdditionalParams);
+
+ // Test another set of correct additional params with principal
+ url =
+ String.format(urlFormatWithPrincipal, quorum1, quorum2, principal,
correctAdditionalParams2);
+ getAndAssertUrlInfo(url, correctAdditionalParams2, principal);
+
+ // Test another set of correct additional params without principal
+ url = String.format(urlFormatWithoutPrincipal, quorum1, quorum2,
correctAdditionalParams2);
+ getAndAssertUrlInfo(url, correctAdditionalParams2);
+
+ // Test another set of correct additional params with principal
+ url =
+ String.format(urlFormatWithPrincipal, quorum1, quorum2, principal,
correctAdditionalParams3);
+ getAndAssertUrlInfo(url, correctAdditionalParams3, principal);
+
+ // Test another set of correct additional params without principal
+ url = String.format(urlFormatWithoutPrincipal, quorum1, quorum2,
correctAdditionalParams3);
+ getAndAssertUrlInfo(url, correctAdditionalParams3);
+
+ // Test incorrect additional params
+ url =
+ String.format(urlFormatWithPrincipal, quorum1, quorum2, principal,
incorrectAdditionalParams);
+ try {
+ getAndAssertUrlInfo(url, incorrectAdditionalParams, principal);
+ } catch (SQLException e) {
+ assertEquals(e.getErrorCode(),
SQLExceptionCode.MALFORMED_CONNECTION_URL.getErrorCode());
+ }
+
+ // Test incorrect additional params without principal
+ url = String.format(urlFormatWithoutPrincipal, quorum1, quorum2,
incorrectAdditionalParams);
+ try {
+ getAndAssertUrlInfo(url, incorrectAdditionalParams);
+ } catch (SQLException e) {
+ assertEquals(e.getErrorCode(),
SQLExceptionCode.MALFORMED_CONNECTION_URL.getErrorCode());
+ }
+
+ // Test missing additional params
+ url =
+ String.format(urlFormatWithPrincipal, quorum1, quorum2, principal,
missingAdditionalParams);
+ try {
+ getAndAssertUrlInfo(url, missingAdditionalParams, principal);
+ } catch (SQLException e) {
+ assertEquals(e.getErrorCode(),
SQLExceptionCode.HA_INVALID_PROPERTIES.getErrorCode());
+ }
+
+ // Test missing additional params without principal
+ url = String.format(urlFormatWithoutPrincipal, quorum1, quorum2,
missingAdditionalParams);
+ try {
+ getAndAssertUrlInfo(url, missingAdditionalParams);
+ } catch (SQLException e) {
+ assertEquals(e.getErrorCode(),
SQLExceptionCode.HA_INVALID_PROPERTIES.getErrorCode());
+ }
+
+ }
+
+ private void getAndAssertUrlInfo(String url, String additionalParams, String
principal)
+ throws Exception {
+ Properties properties = new Properties();
+ HAURLInfo haurlInfo = HighAvailabilityGroup.getUrlInfo(url, properties);
+ assertEquals(haurlInfo.getName(), "testGetUrlInfo");
+ if (principal != null) {
+ assertEquals(haurlInfo.getPrincipal(), principal);
+ } else {
+ assertNull(haurlInfo.getPrincipal());
+ }
+ assertEquals(haurlInfo.getAdditionalJDBCParams(), additionalParams);
+ }
+
+ private void getAndAssertUrlInfo(String url, String additionalParams) throws
Exception {
+ getAndAssertUrlInfo(url, additionalParams, null);
+ }
+}
