Fixes #28726, optimize AlterStorageUnitBackendHandler (#28727)

* Fixes #28726, optimize AlterStorageUnitBackendHandler * Revert ResourceSwitchManager
apache · Oct 12, 2023 · 864b2cd · 864b2cd
1 parent 19eb941
commit 864b2cd
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/...sphere/proxy/backend/handler/distsql/rdl/storage/unit/AlterStorageUnitBackendHandler.java b/...sphere/proxy/backend/handler/distsql/rdl/storage/unit/AlterStorageUnitBackendHandler.java
@@ -27,10 +27,10 @@
 import org.apache.shardingsphere.distsql.segment.URLBasedDataSourceSegment;
 import org.apache.shardingsphere.distsql.segment.converter.DataSourceSegmentsConverter;
 import org.apache.shardingsphere.distsql.statement.rdl.alter.AlterStorageUnitStatement;
+import org.apache.shardingsphere.infra.database.core.connector.ConnectionProperties;
 import org.apache.shardingsphere.infra.database.core.connector.url.JdbcUrl;
 import org.apache.shardingsphere.infra.database.core.connector.url.StandardJdbcUrlParser;
 import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
-import org.apache.shardingsphere.infra.datasource.pool.props.creator.DataSourcePoolPropertiesCreator;
 import org.apache.shardingsphere.infra.datasource.pool.props.domain.DataSourcePoolProperties;
 import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions;
 import org.apache.shardingsphere.infra.exception.core.external.ShardingSphereExternalException;
@@ -40,7 +40,6 @@
 import org.apache.shardingsphere.proxy.backend.response.header.update.UpdateResponseHeader;
 import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
 
-import javax.sql.DataSource;
 import java.sql.SQLException;
 import java.util.Collection;
 import java.util.Collections;
@@ -109,12 +108,12 @@ private void checkStorageUnitNameExisted(final String databaseName, final Collec
     private void checkDatabase(final String databaseName, final AlterStorageUnitStatement sqlStatement) {
         Map<String, StorageUnit> storageUnits = ProxyContext.getInstance().getDatabase(databaseName).getResourceMetaData().getStorageUnits();
         Collection<String> invalidStorageUnitNames = sqlStatement.getStorageUnits().stream().collect(Collectors.toMap(DataSourceSegment::getName, each -> each)).entrySet().stream()
-                .filter(each -> !isIdenticalDatabase(each.getValue(), storageUnits.get(each.getKey()).getDataSource())).map(Entry::getKey).collect(Collectors.toSet());
+                .filter(each -> !isIdenticalDatabase(each.getValue(), storageUnits.get(each.getKey()))).map(Entry::getKey).collect(Collectors.toSet());
         ShardingSpherePreconditions.checkState(invalidStorageUnitNames.isEmpty(),
                 () -> new InvalidStorageUnitsException(Collections.singleton(String.format("Cannot alter the database of %s", invalidStorageUnitNames))));
     }
 
-    private boolean isIdenticalDatabase(final DataSourceSegment segment, final DataSource dataSource) {
+    private boolean isIdenticalDatabase(final DataSourceSegment segment, final StorageUnit storageUnit) {
         String hostName = null;
         String port = null;
         String database = null;
@@ -129,9 +128,8 @@ private boolean isIdenticalDatabase(final DataSourceSegment segment, final DataS
             port = String.valueOf(segmentJdbcUrl.getPort());
             database = segmentJdbcUrl.getDatabase();
         }
-        String url = String.valueOf(DataSourcePoolPropertiesCreator.create(dataSource).getConnectionPropertySynonyms().getStandardProperties().get("url"));
-        JdbcUrl dataSourceJdbcUrl = new StandardJdbcUrlParser().parse(url);
-        return Objects.equals(hostName, dataSourceJdbcUrl.getHostname()) && Objects.equals(port, String.valueOf(dataSourceJdbcUrl.getPort()))
-                && Objects.equals(database, dataSourceJdbcUrl.getDatabase());
+        ConnectionProperties connectionProperties = storageUnit.getConnectionProperties();
+        return Objects.equals(hostName, connectionProperties.getHostname()) && Objects.equals(port, String.valueOf(connectionProperties.getPort()))
+                && Objects.equals(database, connectionProperties.getCatalog());
     }
 }
diff --git a/...re/proxy/backend/handler/distsql/rdl/storage/unit/AlterStorageUnitBackendHandlerTest.java b/...re/proxy/backend/handler/distsql/rdl/storage/unit/AlterStorageUnitBackendHandlerTest.java
@@ -17,7 +17,6 @@
 
 package org.apache.shardingsphere.proxy.backend.handler.distsql.rdl.storage.unit;
 
-import com.zaxxer.hikari.HikariDataSource;
 import org.apache.shardingsphere.distsql.handler.exception.storageunit.DuplicateStorageUnitException;
 import org.apache.shardingsphere.distsql.handler.exception.storageunit.InvalidStorageUnitsException;
 import org.apache.shardingsphere.distsql.handler.exception.storageunit.MissingRequiredStorageUnitsException;
@@ -26,6 +25,7 @@
 import org.apache.shardingsphere.distsql.segment.HostnameAndPortBasedDataSourceSegment;
 import org.apache.shardingsphere.distsql.segment.URLBasedDataSourceSegment;
 import org.apache.shardingsphere.distsql.statement.rdl.alter.AlterStorageUnitStatement;
+import org.apache.shardingsphere.infra.database.core.connector.ConnectionProperties;
 import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
 import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
 import org.apache.shardingsphere.infra.metadata.database.resource.ResourceMetaData;
@@ -81,7 +81,8 @@ void assertExecute() {
         when(ProxyContext.getInstance().getDatabase("foo_db")).thenReturn(database);
         ResourceMetaData resourceMetaData = mock(ResourceMetaData.class, RETURNS_DEEP_STUBS);
         StorageUnit storageUnit = mock(StorageUnit.class, RETURNS_DEEP_STUBS);
-        when(storageUnit.getDataSource()).thenReturn(mockHikariDataSource("ds_0"));
+        ConnectionProperties connectionProperties = mockConnectionProperties("ds_0");
+        when(storageUnit.getConnectionProperties()).thenReturn(connectionProperties);
         when(resourceMetaData.getStorageUnits()).thenReturn(Collections.singletonMap("ds_0", storageUnit));
         when(database.getResourceMetaData()).thenReturn(resourceMetaData);
         assertThat(handler.execute("foo_db", createAlterStorageUnitStatement("ds_0")), instanceOf(UpdateResponseHeader.class));
@@ -108,7 +109,8 @@ void assertExecuteWithAlterDatabase() {
         when(ProxyContext.getInstance().getDatabase("foo_db")).thenReturn(database);
         ResourceMetaData resourceMetaData = mock(ResourceMetaData.class, RETURNS_DEEP_STUBS);
         StorageUnit storageUnit = mock(StorageUnit.class, RETURNS_DEEP_STUBS);
-        when(storageUnit.getDataSource()).thenReturn(mockHikariDataSource("ds_1"));
+        ConnectionProperties connectionProperties = mockConnectionProperties("ds_1");
+        when(storageUnit.getConnectionProperties()).thenReturn(connectionProperties);
         when(resourceMetaData.getStorageUnits()).thenReturn(Collections.singletonMap("ds_0", storageUnit));
         when(database.getResourceMetaData()).thenReturn(resourceMetaData);
         assertThrows(InvalidStorageUnitsException.class, () -> handler.execute("foo_db", createAlterStorageUnitStatement("ds_0")));
@@ -131,9 +133,11 @@ private AlterStorageUnitStatement createAlterStorageUnitStatementWithDuplicateSt
         return new AlterStorageUnitStatement(result);
     }
 
-    private HikariDataSource mockHikariDataSource(final String database) {
-        HikariDataSource result = new HikariDataSource();
-        result.setJdbcUrl(String.format("jdbc:mysql://127.0.0.1:3306/%s?serverTimezone=UTC&useSSL=false", database));
+    private ConnectionProperties mockConnectionProperties(final String catalog) {
+        ConnectionProperties result = mock(ConnectionProperties.class);
+        when(result.getHostname()).thenReturn("127.0.0.1");
+        when(result.getPort()).thenReturn(3306);
+        when(result.getCatalog()).thenReturn(catalog);
         return result;
     }
 }