This is an automated email from the ASF dual-hosted git repository. jianglongtao pushed a commit to branch fix-33341 in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
commit 0e87dcd0d059509fa83fcf700197dd5d9189e8d1 Author: Raigor <[email protected]> AuthorDate: Wed Oct 9 17:20:52 2024 +0800 #5, Optimize show and alter storage units (#13) --- .../AlterStorageUnitConnectionInfoException.java | 4 ++ .../rdl/resource/AlterStorageUnitExecutor.java | 2 +- .../rql/resource/ShowStorageUnitExecutor.java | 52 +++++++++++++--------- .../context/ConfigurationContextManager.java | 25 +++++++---- .../engine/src/main/antlr4/imports/RDLStatement.g4 | 12 ++++- .../core/kernel/KernelDistSQLStatementVisitor.java | 22 +++++++-- .../segment/AlterPoolPropertiesSegment.java | 4 +- .../converter/DataSourceSegmentsConverter.java | 14 +++++- 8 files changed, 95 insertions(+), 40 deletions(-) diff --git a/infra/common/src/main/java/org/apache/shardingsphere/infra/exception/kernel/metadata/resource/storageunit/AlterStorageUnitConnectionInfoException.java b/infra/common/src/main/java/org/apache/shardingsphere/infra/exception/kernel/metadata/resource/storageunit/AlterStorageUnitConnectionInfoException.java index 57e2dd9b64d..0fd249879b1 100644 --- a/infra/common/src/main/java/org/apache/shardingsphere/infra/exception/kernel/metadata/resource/storageunit/AlterStorageUnitConnectionInfoException.java +++ b/infra/common/src/main/java/org/apache/shardingsphere/infra/exception/kernel/metadata/resource/storageunit/AlterStorageUnitConnectionInfoException.java @@ -32,4 +32,8 @@ public final class AlterStorageUnitConnectionInfoException extends ResourceDefin public AlterStorageUnitConnectionInfoException(final Collection<String> storageUnitNames) { super(XOpenSQLState.FEATURE_NOT_SUPPORTED, 11, "Can not alter connection info in storage units: '%s'.", storageUnitNames); } + + public AlterStorageUnitConnectionInfoException(final String message) { + super(XOpenSQLState.FEATURE_NOT_SUPPORTED, 11, message); + } } diff --git a/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java b/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java index 1ae36007c45..b957640345c 100644 --- a/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java +++ b/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rdl/resource/AlterStorageUnitExecutor.java @@ -69,7 +69,7 @@ public final class AlterStorageUnitExecutor implements DistSQLUpdateExecutor<Alt public void executeUpdate(final AlterStorageUnitStatement sqlStatement, final ContextManager contextManager) { checkBefore(sqlStatement); Map<String, DataSourcePoolProperties> propsMap = DataSourceSegmentsConverter.convert(database.getProtocolType(), database.getResourceMetaData(), sqlStatement.getStorageUnits()); - if (!sqlStatement.getStorageUnits().stream().allMatch(each -> each instanceof AlterPoolPropertiesSegment)) { + if (!sqlStatement.getStorageUnits().stream().allMatch(each -> each instanceof AlterPoolPropertiesSegment && null == each.getUser())) { validateHandler.validate(propsMap, getExpectedPrivileges(sqlStatement)); } try { diff --git a/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rql/resource/ShowStorageUnitExecutor.java b/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rql/resource/ShowStorageUnitExecutor.java index 2b3d879d2ee..745324fa046 100644 --- a/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rql/resource/ShowStorageUnitExecutor.java +++ b/infra/distsql-handler/src/main/java/org/apache/shardingsphere/distsql/handler/executor/rql/resource/ShowStorageUnitExecutor.java @@ -23,7 +23,6 @@ import org.apache.shardingsphere.distsql.handler.aware.DistSQLExecutorDatabaseAw import org.apache.shardingsphere.distsql.handler.engine.query.DistSQLQueryExecutor; import org.apache.shardingsphere.distsql.statement.rql.resource.ShowStorageUnitsStatement; import org.apache.shardingsphere.infra.database.core.connector.ConnectionProperties; -import org.apache.shardingsphere.infra.database.core.type.DatabaseTypeRegistry; import org.apache.shardingsphere.infra.datasource.pool.CatalogSwitchableDataSource; import org.apache.shardingsphere.infra.datasource.pool.props.creator.DataSourcePoolPropertiesCreator; import org.apache.shardingsphere.infra.datasource.pool.props.domain.DataSourcePoolProperties; @@ -59,7 +58,7 @@ public final class ShowStorageUnitExecutor implements DistSQLQueryExecutor<ShowS @Override public Collection<String> getColumnNames(final ShowStorageUnitsStatement sqlStatement) { return Arrays.asList("name", "type", "host", "port", "db", "connection_timeout_milliseconds", "idle_timeout_milliseconds", - "max_lifetime_milliseconds", "max_pool_size", "min_pool_size", "read_only", "other_attributes", "pool_name"); + "max_lifetime_milliseconds", "max_pool_size", "min_pool_size", "read_only", "other_attributes", "username", "pool_name", "actual_jdbc_url"); } @SphereEx(Type.MODIFY) @@ -68,7 +67,9 @@ public final class ShowStorageUnitExecutor implements DistSQLQueryExecutor<ShowS Collection<LocalDataQueryResultRow> result = new LinkedList<>(); for (Entry<String, StorageUnit> entry : getToBeShownStorageUnits(sqlStatement).entrySet()) { ConnectionProperties connectionProps = entry.getValue().getConnectionProperties(); - DataSourcePoolProperties dataSourcePoolProps = getDataSourcePoolProperties(entry.getValue()); + @SphereEx + DataSource actualDataSource = getActualDataSource(entry.getValue().getDataSource()); + DataSourcePoolProperties dataSourcePoolProps = getDataSourcePoolProperties(actualDataSource); Map<String, Object> poolProps = dataSourcePoolProps.getPoolPropertySynonyms().getStandardProperties(); Map<String, Object> customProps = getCustomProperties(dataSourcePoolProps.getCustomProperties().getProperties(), connectionProps.getQueryProperties()); result.add(new LocalDataQueryResultRow(entry.getKey(), @@ -82,7 +83,8 @@ public final class ShowStorageUnitExecutor implements DistSQLQueryExecutor<ShowS getStandardProperty(poolProps, "maxPoolSize"), getStandardProperty(poolProps, "minPoolSize"), getStandardProperty(poolProps, "readOnly"), - customProps, getPoolName(entry.getValue().getDataSource()))); + customProps, getUsername(actualDataSource, dataSourcePoolProps.getAllStandardProperties()), + getPoolName(actualDataSource), getActualURL(actualDataSource))); } return result; } @@ -108,18 +110,8 @@ public final class ShowStorageUnitExecutor implements DistSQLQueryExecutor<ShowS return result; } - private DataSourcePoolProperties getDataSourcePoolProperties(final StorageUnit storageUnit) { - DataSource dataSource = storageUnit.getDataSource(); - DataSourcePoolProperties result = DataSourcePoolPropertiesCreator.create( - dataSource instanceof CatalogSwitchableDataSource ? ((CatalogSwitchableDataSource) dataSource).getDataSource() : dataSource); - if (new DatabaseTypeRegistry(storageUnit.getStorageType()).getDialectDatabaseMetaData().isInstanceConnectionAvailable()) { - for (Entry<String, Object> entry : storageUnit.getDataSourcePoolProperties().getPoolPropertySynonyms().getStandardProperties().entrySet()) { - if (null != entry.getValue()) { - result.getPoolPropertySynonyms().getStandardProperties().put(entry.getKey(), entry.getValue()); - } - } - } - return result; + private DataSourcePoolProperties getDataSourcePoolProperties(final DataSource actualDataSource) { + return DataSourcePoolPropertiesCreator.create(actualDataSource); } private Map<String, Object> getCustomProperties(final Map<String, Object> customProps, final Properties queryProps) { @@ -132,10 +124,30 @@ public final class ShowStorageUnitExecutor implements DistSQLQueryExecutor<ShowS } @SphereEx - private String getPoolName(final DataSource dataSource) { - DataSource realDataSource = dataSource instanceof CatalogSwitchableDataSource ? ((CatalogSwitchableDataSource) dataSource).getDataSource() : dataSource; - if (realDataSource instanceof HikariDataSource) { - return ((HikariDataSource) realDataSource).getPoolName(); + private DataSource getActualDataSource(final DataSource dataSource) { + return dataSource instanceof CatalogSwitchableDataSource ? ((CatalogSwitchableDataSource) dataSource).getDataSource() : dataSource; + } + + @SphereEx + private String getUsername(final DataSource actualDataSource, final Map<String, Object> standardProps) { + if (actualDataSource instanceof HikariDataSource) { + return ((HikariDataSource) actualDataSource).getUsername(); + } + return getStandardProperty(standardProps, "username"); + } + + @SphereEx + private String getPoolName(final DataSource actualDataSource) { + if (actualDataSource instanceof HikariDataSource) { + return ((HikariDataSource) actualDataSource).getPoolName(); + } + return ""; + } + + @SphereEx + private String getActualURL(final DataSource actualDataSource) { + if (actualDataSource instanceof HikariDataSource) { + return ((HikariDataSource) actualDataSource).getJdbcUrl(); } return ""; } diff --git a/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/context/ConfigurationContextManager.java b/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/context/ConfigurationContextManager.java index a4c5db3a570..b570b6006e5 100644 --- a/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/context/ConfigurationContextManager.java +++ b/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/context/ConfigurationContextManager.java @@ -108,7 +108,7 @@ public final class ConfigurationContextManager { new ResourceSwitchManager().alterStorageUnit(metaDataContexts.get().getMetaData().getDatabase(databaseName).getResourceMetaData(), propsMap); // SPEX ADDED: BEGIN ResourceMetaData resourceMetaData = metaDataContexts.get().getMetaData().getDatabase(databaseName).getResourceMetaData(); - if (isPoolPropsChangedOnly(resourceMetaData, propsMap)) { + if (canBeChangedInRuntime(resourceMetaData, propsMap)) { alterDataSourcePoolPros(resourceMetaData, switchingResource, propsMap); closeNewDataSources(switchingResource); return; @@ -121,22 +121,19 @@ public final class ConfigurationContextManager { } @SphereEx - private boolean isPoolPropsChangedOnly(final ResourceMetaData resourceMetaData, final Map<String, DataSourcePoolProperties> propsMap) { + private boolean canBeChangedInRuntime(final ResourceMetaData resourceMetaData, final Map<String, DataSourcePoolProperties> propsMap) { for (Entry<String, DataSourcePoolProperties> entry : propsMap.entrySet()) { if (!resourceMetaData.getStorageUnits().containsKey(entry.getKey())) { continue; } + if (!entry.getValue().getCustomProperties().getProperties().containsKey("alterInRuntime")) { + return false; + } ConnectionPropertySynonyms currentConnectionProps = resourceMetaData.getStorageUnits().get(entry.getKey()).getDataSourcePoolProperties().getConnectionPropertySynonyms(); ConnectionPropertySynonyms newConnectionProps = entry.getValue().getConnectionPropertySynonyms(); if (!currentConnectionProps.getStandardProperties().get("url").equals(newConnectionProps.getStandardProperties().get("url"))) { return false; } - if (!currentConnectionProps.getStandardProperties().get("username").equals(newConnectionProps.getStandardProperties().get("username"))) { - return false; - } - if (!currentConnectionProps.getStandardProperties().get("password").equals(newConnectionProps.getStandardProperties().get("password"))) { - return false; - } } return true; } @@ -154,7 +151,8 @@ public final class ConfigurationContextManager { DataSource newDataSource = switchingResource.getNewDataSources().get(storageUnit.getStorageNode()); ShardingSpherePreconditions.checkState(newDataSource instanceof HikariDataSource, () -> new UnsupportedOperationException("The new data source must be HikariDataSource when alter pool properties.")); - if (!((HikariDataSource) newDataSource).getJdbcUrl().equals(entry.getValue().getConnectionPropertySynonyms().getStandardProperties().get("url"))) { + Map<String, Object> standardProperties = entry.getValue().getConnectionPropertySynonyms().getStandardProperties(); + if (!((HikariDataSource) newDataSource).getJdbcUrl().equals(standardProperties.get("url"))) { continue; } DataSource staleDataSource = switchingResource.getStaleDataSources().get(storageUnit.getStorageNode()); @@ -185,6 +183,11 @@ public final class ConfigurationContextManager { log.warn("Update maxLifetimeMs of pool `{}` from {} to {}", configMXBean.getPoolName(), configMXBean.getMaxLifetime(), newConfigMXBean.getMaxLifetime()); configMXBean.setMaxLifetime(newConfigMXBean.getMaxLifetime()); } + if (isUsernameOrPasswordChanged((HikariDataSource) newDataSource, (HikariDataSource) staleDataSource)) { + configMXBean.setUsername(((HikariDataSource) newDataSource).getUsername()); + configMXBean.setPassword(((HikariDataSource) newDataSource).getPassword()); + log.warn("Update username and password of pool `{}`", configMXBean.getPoolName()); + } } for (Entry<String, StorageUnit> entry : resourceMetaData.getStorageUnits().entrySet()) { DataSourcePoolProperties newDataSourceProperties = switchingResource.getMergedDataSourcePoolPropertiesMap().get(entry.getKey()); @@ -194,6 +197,10 @@ public final class ConfigurationContextManager { } } + private boolean isUsernameOrPasswordChanged(final HikariDataSource newDataSource, final HikariDataSource staleDataSource) { + return !newDataSource.getUsername().equals(staleDataSource.getUsername()) || !newDataSource.getPassword().equals(staleDataSource.getPassword()); + } + @SphereEx private void alterDataSourceProperties(final StorageUnit storageUnit, final DataSourcePoolProperties newDataSourceProperties) { storageUnit.getDataSourcePoolProperties().getPoolPropertySynonyms().getStandardProperties().clear(); diff --git a/parser/distsql/engine/src/main/antlr4/imports/RDLStatement.g4 b/parser/distsql/engine/src/main/antlr4/imports/RDLStatement.g4 index f65a26f1e3f..2f4fa6b5d61 100644 --- a/parser/distsql/engine/src/main/antlr4/imports/RDLStatement.g4 +++ b/parser/distsql/engine/src/main/antlr4/imports/RDLStatement.g4 @@ -36,11 +36,19 @@ storageUnitsDefinition ; storageUnitDefinition - : storageUnitName LP_ storageUnitConnectionDefinition? (COMMA_? propertiesDefinition)? RP_ + : storageUnitName LP_ (storageUnitConnectionDefinition | alterPoolPropertiesDefinition) RP_ ; storageUnitConnectionDefinition - : (simpleSource | urlSource) COMMA_ USER EQ_ user (COMMA_ PASSWORD EQ_ password)? + : (simpleSource | urlSource) COMMA_ userAndPassword (COMMA_ propertiesDefinition)? + ; + +alterPoolPropertiesDefinition + : userAndPassword? (COMMA_? propertiesDefinition)? + ; + +userAndPassword + : USER EQ_ user (COMMA_ PASSWORD EQ_ password)? ; simpleSource diff --git a/parser/distsql/engine/src/main/java/org/apache/shardingsphere/distsql/parser/core/kernel/KernelDistSQLStatementVisitor.java b/parser/distsql/engine/src/main/java/org/apache/shardingsphere/distsql/parser/core/kernel/KernelDistSQLStatementVisitor.java index 0b81452b968..352157a365a 100644 --- a/parser/distsql/engine/src/main/java/org/apache/shardingsphere/distsql/parser/core/kernel/KernelDistSQLStatementVisitor.java +++ b/parser/distsql/engine/src/main/java/org/apache/shardingsphere/distsql/parser/core/kernel/KernelDistSQLStatementVisitor.java @@ -64,6 +64,7 @@ import org.apache.shardingsphere.distsql.parser.autogen.KernelDistSQLStatementPa import org.apache.shardingsphere.distsql.parser.autogen.KernelDistSQLStatementParser.UnlabelComputeNodeContext; import org.apache.shardingsphere.distsql.parser.autogen.KernelDistSQLStatementParser.UnlockClusterContext; import org.apache.shardingsphere.distsql.parser.autogen.KernelDistSQLStatementParser.UnregisterStorageUnitContext; +import org.apache.shardingsphere.distsql.parser.autogen.KernelDistSQLStatementParser.UserAndPasswordContext; import org.apache.shardingsphere.distsql.segment.AlgorithmSegment; import org.apache.shardingsphere.distsql.segment.AlterPoolPropertiesSegment; import org.apache.shardingsphere.distsql.segment.DataSourceSegment; @@ -143,13 +144,13 @@ public final class KernelDistSQLStatementVisitor extends KernelDistSQLStatementB @SphereEx(Type.MODIFY) @Override public ASTNode visitStorageUnitDefinition(final StorageUnitDefinitionContext ctx) { - Properties props = getProperties(ctx.propertiesDefinition()); if (null == ctx.storageUnitConnectionDefinition()) { - return new AlterPoolPropertiesSegment(getIdentifierValue(ctx.storageUnitName()), props); + return getAlterPoolPropertiesSegment(ctx); } StorageUnitConnectionDefinitionContext connectionCtx = ctx.storageUnitConnectionDefinition(); - String user = getIdentifierValue(connectionCtx.user()); - String password = null == connectionCtx.password() ? "" : getPassword(connectionCtx.password()); + String user = getIdentifierValue(connectionCtx.userAndPassword().user()); + String password = null == connectionCtx.userAndPassword().password() ? "" : getPassword(connectionCtx.userAndPassword().password()); + Properties props = getProperties(connectionCtx.propertiesDefinition()); return null == connectionCtx.urlSource() ? new HostnameAndPortBasedDataSourceSegment(getIdentifierValue(ctx.storageUnitName()), getIdentifierValue(connectionCtx.simpleSource().hostname()), connectionCtx.simpleSource().port().getText(), @@ -157,6 +158,19 @@ public final class KernelDistSQLStatementVisitor extends KernelDistSQLStatementB : new URLBasedDataSourceSegment(getIdentifierValue(ctx.storageUnitName()), getIdentifierValue(connectionCtx.urlSource().url()), user, password, props); } + @SphereEx + private AlterPoolPropertiesSegment getAlterPoolPropertiesSegment(final StorageUnitDefinitionContext ctx) { + String user = null; + String password = null; + UserAndPasswordContext userAndPasswordContext = ctx.alterPoolPropertiesDefinition().userAndPassword(); + if (null != userAndPasswordContext) { + user = getIdentifierValue(userAndPasswordContext.user()); + password = null == userAndPasswordContext.password() ? "" : getPassword(userAndPasswordContext.password()); + } + Properties props = getProperties(ctx.alterPoolPropertiesDefinition().propertiesDefinition()); + return new AlterPoolPropertiesSegment(getIdentifierValue(ctx.storageUnitName()), user, password, props); + } + private String getPassword(final PasswordContext ctx) { return null == ctx ? null : StringLiteralValue.getStandardEscapesStringLiteralValue(ctx.getText()).getValue(); } diff --git a/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/AlterPoolPropertiesSegment.java b/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/AlterPoolPropertiesSegment.java index 935c61179be..c639c66fd94 100644 --- a/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/AlterPoolPropertiesSegment.java +++ b/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/AlterPoolPropertiesSegment.java @@ -29,7 +29,7 @@ import java.util.Properties; @Getter public final class AlterPoolPropertiesSegment extends DataSourceSegment { - public AlterPoolPropertiesSegment(final String name, final Properties props) { - super(name, null, null, props); + public AlterPoolPropertiesSegment(final String name, final String user, final String password, final Properties props) { + super(name, user, password, props); } } diff --git a/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/converter/DataSourceSegmentsConverter.java b/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/converter/DataSourceSegmentsConverter.java index 23469dfaade..8bc0ef5e7b1 100644 --- a/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/converter/DataSourceSegmentsConverter.java +++ b/parser/distsql/statement/src/main/java/org/apache/shardingsphere/distsql/segment/converter/DataSourceSegmentsConverter.java @@ -26,6 +26,8 @@ import org.apache.shardingsphere.distsql.segment.HostnameAndPortBasedDataSourceS import org.apache.shardingsphere.distsql.segment.URLBasedDataSourceSegment; import org.apache.shardingsphere.infra.database.core.type.DatabaseType; import org.apache.shardingsphere.infra.datasource.pool.props.domain.DataSourcePoolProperties; +import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions; +import org.apache.shardingsphere.infra.exception.kernel.metadata.resource.storageunit.AlterStorageUnitConnectionInfoException; import org.apache.shardingsphere.infra.metadata.database.resource.ResourceMetaData; import org.apache.shardingsphere.infra.metadata.database.resource.unit.StorageUnit; import org.apache.shardingsphere.infra.util.SphereEx; @@ -83,6 +85,8 @@ public final class DataSourceSegmentsConverter { @SphereEx private static DataSourcePoolProperties convertForAlterPoolProps(final ResourceMetaData resourceMetaData, final AlterPoolPropertiesSegment segment) { + ShardingSpherePreconditions.checkState(!segment.getProps().containsKey("readOnly"), + () -> new AlterStorageUnitConnectionInfoException(String.format("Can not set `readOnly` when just alter pool properties for storage unit `%s`.", segment.getName()))); StorageUnit storageUnit = resourceMetaData.getStorageUnits().get(segment.getName()); Map<String, Object> props = getAlteredPoolProps(getCurrentProps(storageUnit.getDataSourcePoolProperties().getAllStandardProperties()), segment); return new DataSourcePoolProperties("com.zaxxer.hikari.HikariDataSource", props); @@ -101,11 +105,17 @@ public final class DataSourceSegmentsConverter { @SphereEx private static Map<String, Object> getAlteredPoolProps(final Map<String, Object> props, final AlterPoolPropertiesSegment segment) { + props.put("alterInRuntime", true); + if (null != segment.getUser()) { + props.put("username", segment.getUser()); + props.put("password", segment.getPassword()); + log.warn("Alter username and password for storage unit `{}`, username: {}", segment.getName(), segment.getUser()); + } Properties toBeAlteredProperties = segment.getProps(); if (toBeAlteredProperties.isEmpty()) { return props; } - log.warn("Alter pool properties for storage unit {}, props: {}", segment.getName(), toBeAlteredProperties); + log.warn("Alter pool properties for storage unit `{}`, props: {}", segment.getName(), toBeAlteredProperties); putPropsIfPresent(toBeAlteredProperties, props, "maxPoolSize"); putPropsIfPresent(toBeAlteredProperties, props, "minPoolSize"); putPropsIfPresent(toBeAlteredProperties, props, "connectionTimeoutMilliseconds"); @@ -131,7 +141,7 @@ public final class DataSourceSegmentsConverter { if (toBeAlteredProperties.containsKey(key)) { props.put(key, toBeAlteredProperties.getProperty(key)); } else if (toBeAlteredProperties.containsKey(propertySynonyms.get(key))) { - props.put(key, toBeAlteredProperties.get(propertySynonyms.get(key))); + props.put(key, toBeAlteredProperties.getProperty(propertySynonyms.get(key))); } }
