strongduanmu commented on code in PR #20770: URL: https://github.com/apache/shardingsphere/pull/20770#discussion_r970206543
########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/state/DataSourceState.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.shardingsphere.infra.datasource.state; + +/** + * Data source state. + */ +public enum DataSourceState { + + DISABLED, ENABLED; + + /** + * Data source disable or enable. + * + * @param status data source state + * @return disable or enable + */ + public static boolean isDisable(final String status) { + return DISABLED.name().toLowerCase().equals(status); + } + + /** + * Data source disable or enable. + * + * @param status storage node status + * @return disable or enable + */ + public static boolean isEnable(final String status) { + return ENABLED.name().toLowerCase().equals(status); + } + + /** + * Get data source state by state name. + * + * @param state data source state name + * @return data source state + */ + public static DataSourceState getDataSourceState(final String state) { Review Comment: Is it possible to use map to improve performance? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/state/DataSourceState.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.shardingsphere.infra.datasource.state; + +/** + * Data source state. + */ +public enum DataSourceState { + + DISABLED, ENABLED; + + /** + * Data source disable or enable. + * + * @param status data source state + * @return disable or enable + */ + public static boolean isDisable(final String status) { + return DISABLED.name().toLowerCase().equals(status); Review Comment: Do you think equalsIgnoreCase is better? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/state/DataSourceState.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.shardingsphere.infra.datasource.state; + +/** + * Data source state. + */ +public enum DataSourceState { + + DISABLED, ENABLED; + + /** + * Data source disable or enable. + * + * @param status data source state + * @return disable or enable + */ + public static boolean isDisable(final String status) { + return DISABLED.name().toLowerCase().equals(status); + } + + /** + * Data source disable or enable. + * + * @param status storage node status + * @return disable or enable + */ + public static boolean isEnable(final String status) { + return ENABLED.name().toLowerCase().equals(status); Review Comment: Do you think equalsIgnoreCase is better? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeEngine.java: ########## @@ -64,9 +66,7 @@ public static DatabaseType getProtocolType(final Map<String, ? extends DatabaseC if (configuredDatabaseType.isPresent()) { return configuredDatabaseType.get(); } - Collection<DataSource> dataSources = databaseConfigs.values().stream() - .filter(each -> !each.getDataSources().isEmpty()).findFirst().map(optional -> optional.getDataSources().values()).orElseGet(Collections::emptyList); - return getDatabaseType(dataSources); + return getDatabaseType(DataSourceStateManager.getInstance().getEnabledDataSources(databaseConfigs)); Review Comment: Why is the databaseName parameter not needed here? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/state/DataSourceStateManager.java: ########## @@ -0,0 +1,151 @@ +/* + * 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.shardingsphere.infra.datasource.state; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.shardingsphere.infra.config.database.DatabaseConfiguration; +import org.apache.shardingsphere.infra.datasource.state.exception.DataSourceStateException; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Data source state manager. + */ +@Slf4j +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public final class DataSourceStateManager { + + private static final DataSourceStateManager INSTANCE = new DataSourceStateManager(); + + private final Map<String, DataSourceState> dataSourceStates = new ConcurrentHashMap<>(); + + private volatile boolean force; + + private volatile boolean initialized; + + /** + * Get data source state manager. + * + * @return data source state manager + */ + public static DataSourceStateManager getInstance() { + return INSTANCE; + } + + /** + * Set data source states when bootstrap. + * + * @param databaseName database name + * @param dataSources data sources + * @param storageDataSourceStates storage node data source status + * @param force whether to force start + */ + public void initStates(final String databaseName, final Map<String, DataSource> dataSources, final Map<String, DataSourceState> storageDataSourceStates, final boolean force) { + this.force = force; + dataSources.forEach((key, value) -> { + DataSourceState storageState = storageDataSourceStates.get(getCacheKey(databaseName, key)); + if (DataSourceState.DISABLED == storageState) { + dataSourceStates.put(getCacheKey(databaseName, key), storageState); + } else { + try (Connection ignored = value.getConnection()) { + dataSourceStates.put(getCacheKey(databaseName, key), DataSourceState.ENABLED); + } catch (final SQLException ex) { + if (this.force) { + log.error("Data source status unavailable, ignored with the -f parameter.", ex); + } else { + throw new DataSourceStateException("DataSourceState", 1, "Data source status unavailable.", ex); + } + } + } + }); + initialized = true; + } + + /** + * Get enabled data sources. + * + * @param databaseConfigs database config + * @return enabled data sources + */ + public Collection<DataSource> getEnabledDataSources(final Map<String, ? extends DatabaseConfiguration> databaseConfigs) { Review Comment: databaseConfigs represents multiple logical libraries, is there a problem with the processing logic here? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/metadata/jdbc/JDBCInstanceMetaData.java: ########## @@ -32,9 +32,18 @@ public final class JDBCInstanceMetaData implements InstanceMetaData { private final String ip; + private final boolean force; Review Comment: @zhaojinchao95 Can you help review this change? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/database/resource/ShardingSphereResource.java: ########## @@ -39,16 +40,19 @@ @Getter public final class ShardingSphereResource { + private final String databaseName; Review Comment: Why add databaseName here? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/state/DataSourceStateManager.java: ########## @@ -0,0 +1,151 @@ +/* + * 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.shardingsphere.infra.datasource.state; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.shardingsphere.infra.config.database.DatabaseConfiguration; +import org.apache.shardingsphere.infra.datasource.state.exception.DataSourceStateException; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Data source state manager. + */ +@Slf4j +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public final class DataSourceStateManager { + + private static final DataSourceStateManager INSTANCE = new DataSourceStateManager(); + + private final Map<String, DataSourceState> dataSourceStates = new ConcurrentHashMap<>(); + + private volatile boolean force; + + private volatile boolean initialized; + + /** + * Get data source state manager. + * + * @return data source state manager + */ + public static DataSourceStateManager getInstance() { + return INSTANCE; + } + + /** + * Set data source states when bootstrap. + * + * @param databaseName database name + * @param dataSources data sources + * @param storageDataSourceStates storage node data source status + * @param force whether to force start + */ + public void initStates(final String databaseName, final Map<String, DataSource> dataSources, final Map<String, DataSourceState> storageDataSourceStates, final boolean force) { + this.force = force; + dataSources.forEach((key, value) -> { Review Comment: Is it possible to reduce the hierarchy by splitting the method? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/metadata/proxy/ProxyInstanceMetaData.java: ########## @@ -37,17 +37,28 @@ public final class ProxyInstanceMetaData implements InstanceMetaData { private final int port; + private final boolean force; + Review Comment: @zhaojinchao95 Can you help review this change? ########## shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/database/type/DatabaseTypeEngine.java: ########## @@ -44,12 +44,14 @@ public final class DatabaseTypeEngine { /** * Get protocol type. * + * Review Comment: Please remove this useless line. ########## shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/cluster/ClusterContextManagerBuilder.java: ########## @@ -54,13 +63,36 @@ public ContextManager build(final ContextManagerBuilderParameter parameter) thro if (persistRepository instanceof InstanceContextAware) { ((InstanceContextAware) persistRepository).setInstanceContext(instanceContext); } + checkDataSourceStates(parameter.getDatabaseConfigs(), registryCenter, parameter.getInstanceMetaData().isForce()); Review Comment: @zhaojinchao95 Can you help review this change? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
