You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/07/15 09:02:50 UTC

[GitHub] [shardingsphere] Beyondeclipse opened a new pull request #11345: improve loading TableMetaData by Multi-Thread

Beyondeclipse opened a new pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345


   for #11028.
   
   Changes proposed in this pull request:
   - ignore 'ExistedTables' and loadTableMetaData by Multi-Thread
   
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r674710729



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -129,16 +120,40 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         }
         return Optional.empty();
     }
-    
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+
+    private static Map<String, String> getActualTable2LogicTableMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
+        }
+
+        int size = dataNodeContainedRuleList.stream()
+                .mapToInt(each -> each.getAllDataNodes().entrySet().stream().mapToInt(entry -> entry.getValue().size()).sum())
+                .sum();
+        Map<String, String> actualTable2LogicTableMap = new HashMap<>(size);
+        for (DataNodeContainedRule each : dataNodeContainedRuleList) {
+            for (Entry<String, Collection<DataNode>> entry : each.getAllDataNodes().entrySet()) {
+                List<String> actualTables = entry.getValue().stream().map(DataNode::getTableName).collect(Collectors.toList());
+                for (String oneActualTalbe : actualTables) {
+                    actualTable2LogicTableMap.put(oneActualTalbe, entry.getKey());
+                }
+            }
+        }
+        return actualTable2LogicTableMap;
+    }
+
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials,
+            final Map<String, String> ruleActualTable2LogicTableMap, final Map<String, TableMetaData> tables) throws SQLException {
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each)));

Review comment:
       @Beyondeclipse This change will cause too many sharding actual tables to be loaded, and it is difficult to improve performance when there are too many sharding tables.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse closed pull request #11345: Improve loading TableMetaData for DialectTableMetaDataLoader

Posted by GitBox <gi...@apache.org>.

Beyondeclipse closed pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345


   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880537934


   @tristaZero @totalo , Hi guys, I found method SchemaBuilder#appendDialectRemainTables gets exists table names and excludes them in loading TableMetaData , while it seems not necessary to do this. And I fix this and modify the loading data from 'TableContainedRule' by multi-thread, And in my system envirement, which has hundreds of databases, it makes the startup faster.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r673878349



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +141,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));
         }
         for (Future<Map<String, TableMetaData>> each : futures) {
             try {
-                tables.putAll(each.get());
+                Map<String, TableMetaData> tableMetaMap = each.get();
+                for (Map.Entry<String, TableMetaData> one : tableMetaMap.entrySet()) {
+                    String actualTableName = one.getKey();
+                    String logicTableName = actualTable2LogicTableMap.get(actualTableName);

Review comment:
       Maybe it's better to extract this code into a method.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       @Beyondeclipse If it is no longer needed, please delete this parameter.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +141,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));
         }
         for (Future<Map<String, TableMetaData>> each : futures) {
             try {
-                tables.putAll(each.get());
+                Map<String, TableMetaData> tableMetaMap = each.get();
+                for (Map.Entry<String, TableMetaData> one : tableMetaMap.entrySet()) {

Review comment:
       Please modify param name according official code conduct.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -147,9 +173,35 @@ private static void appendDialectRemainTables(final DialectTableMetaDataLoader d
             }
         }
     }
-    
+
+    private static Map<String, String> getActualTable2LogicTableMap(final SchemaBuilderMaterials materials, final Collection<String> ruleLogicTables) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
+        }
+
+        int size = dataNodeContainedRuleList.stream()
+                .mapToInt(each -> each.getAllDataNodes().entrySet().stream()
+                        .mapToInt(entry -> entry.getValue().size())
+                        .sum())
+                .sum();
+        Map<String, String> actualTable2LogicTableMap = new HashMap<>(size);

Review comment:
       Please refer official code conduct to change the code style.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       @Beyondeclipse If you adjust the order of execution, the result parameter will contain the rule table.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tristaZero commented on pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

tristaZero commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-893238120


   Hi @Beyondeclipse Thanks for this PR, I know it will be a big task but definitely an important enhancement. If any issues, welcome your discussion with @strongduanmu . We will provide the assistance we can.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-883314839


   > @Beyondeclipse Please add test case for your change.
   I will do it later.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9f8cc84) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **decrease** coverage by `0.22%`.
   > The diff coverage is `82.60%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   - Coverage     63.44%   63.22%   -0.23%     
   - Complexity     1028     1091      +63     
   ============================================
     Files          2216     2224       +8     
     Lines         33753    34072     +319     
     Branches       5897     5936      +39     
   ============================================
   + Hits          21416    21543     +127     
   - Misses        10629    10804     +175     
   - Partials       1708     1725      +17     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ilder/loader/dialect/MySQLTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9NeVNRTFRhYmxlTWV0YURhdGFMb2FkZXIuamF2YQ==) | `85.45% <0.00%> (-1.34%)` | :arrow_down: |
   | [...lder/loader/dialect/OracleTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9PcmFjbGVUYWJsZU1ldGFEYXRhTG9hZGVyLmphdmE=) | `84.93% <0.00%> (-1.18%)` | :arrow_down: |
   | [.../loader/dialect/PostgreSQLTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9Qb3N0Z3JlU1FMVGFibGVNZXRhRGF0YUxvYWRlci5qYXZh) | `84.37% <ø> (-1.34%)` | :arrow_down: |
   | [...r/loader/dialect/SQLServerTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9TUUxTZXJ2ZXJUYWJsZU1ldGFEYXRhTG9hZGVyLmphdmE=) | `84.90% <0.00%> (-1.64%)` | :arrow_down: |
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `96.77% <ø> (ø)` | |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `88.61% <86.15%> (+19.38%)` | :arrow_up: |
   | [.../builder/loader/dialect/H2TableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9IMlRhYmxlTWV0YURhdGFMb2FkZXIuamF2YQ==) | `87.20% <100.00%> (+0.15%)` | :arrow_up: |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `0.00% <0.00%> (-88.24%)` | :arrow_down: |
   | [...ere/agent/metrics/api/advice/DataSourceAdvice.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtYWdlbnQvc2hhcmRpbmdzcGhlcmUtYWdlbnQtcGx1Z2lucy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1wbHVnaW4tbWV0cmljcy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYWdlbnQvbWV0cmljcy9hcGkvYWR2aWNlL0RhdGFTb3VyY2VBZHZpY2UuamF2YQ==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [...onfig/subscriber/GlobalRuleRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9jb25maWcvc3Vic2NyaWJlci9HbG9iYWxSdWxlUmVnaXN0cnlTdWJzY3JpYmVyLmphdmE=) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | ... and [123 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...9f8cc84](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-887984472


   @Beyondeclipse  By the way, this PR will affect the `check-table-metadata-enabled` function. It is recommended to take out the load optimization logic of the rule table separately.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d179290) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.07%`.
   > The diff coverage is `86.36%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   + Coverage     63.44%   63.52%   +0.07%     
   - Complexity     1028     1090      +62     
   ============================================
     Files          2216     2220       +4     
     Lines         33753    33882     +129     
     Branches       5897     5908      +11     
   ============================================
   + Hits          21416    21523     +107     
   - Misses        10629    10643      +14     
   - Partials       1708     1716       +8     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `96.77% <ø> (ø)` | |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `88.70% <86.36%> (+19.47%)` | :arrow_up: |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `0.00% <0.00%> (-88.24%)` | :arrow_down: |
   | [...onfig/subscriber/GlobalRuleRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9jb25maWcvc3Vic2NyaWJlci9HbG9iYWxSdWxlUmVnaXN0cnlTdWJzY3JpYmVyLmphdmE=) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | [...ra/route/engine/single/SingleTableRouteEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtcm91dGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL3JvdXRlL2VuZ2luZS9zaW5nbGUvU2luZ2xlVGFibGVSb3V0ZUVuZ2luZS5qYXZh) | `57.14% <0.00%> (-24.68%)` | :arrow_down: |
   | [...rnance/core/registry/GovernanceWatcherFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9Hb3Zlcm5hbmNlV2F0Y2hlckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (-13.34%)` | :arrow_down: |
   | [.../process/subscriber/ProcessRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9wcm9jZXNzL3N1YnNjcmliZXIvUHJvY2Vzc1JlZ2lzdHJ5U3Vic2NyaWJlci5qYXZh) | `0.00% <0.00%> (-12.91%)` | :arrow_down: |
   | [.../postgresql/codec/PostgreSQLPacketCodecEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGIvcHJvdG9jb2wvcG9zdGdyZXNxbC9jb2RlYy9Qb3N0Z3JlU1FMUGFja2V0Q29kZWNFbmdpbmUuamF2YQ==) | `63.33% <0.00%> (-9.09%)` | :arrow_down: |
   | [...e/validator/ddl/ShardingDDLStatementValidator.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3ZhbGlkYXRvci9kZGwvU2hhcmRpbmdERExTdGF0ZW1lbnRWYWxpZGF0b3IuamF2YQ==) | `95.45% <0.00%> (-4.55%)` | :arrow_down: |
   | [.../route/engine/type/ShardingRouteEngineFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3R5cGUvU2hhcmRpbmdSb3V0ZUVuZ2luZUZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-4.26%)` | :arrow_down: |
   | ... and [101 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...d179290](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tuichenchuxin commented on a change in pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

tuichenchuxin commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r683907218



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -66,119 +59,114 @@
     }
     
     /**
-     * build actual and logic table meta data.
+     * Build actual and logic table metadata.
      *
      * @param materials schema builder materials
-     * @return actual and logic table meta data
+     * @return actual and logic table metadata
      * @throws SQLException SQL exception
      */
     public static Map<TableMetaData, TableMetaData> build(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = getLogicTableDataNodesMap(materials);
+        Optional<DialectTableMetaDataLoader> dialectLoader = SchemaBuilderWithDialectLoader.findDialectTableMetaDataLoader(materials);
+        Map<String, TableMetaData> result;
+        if (dialectLoader.isPresent()) {
+            result = SchemaBuilderWithDialectLoader.build(dialectLoader.get(), EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
+        } else {
+            result = SchemaBuilderWithDefaultLoader.build(EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
-        if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
-            return;
+    private static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
         }
-        appendDefaultRemainTables(materials, tables);
-    }
-    
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (tables.containsKey(table)) {
-                        TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
-                        result.put(table, metaData);
-                    }
-                }
-            }
+
+        Map<String, Collection<DataNode>> logicTableDataNodes = getLogicTableDataNodesMap(dataNodeContainedRuleList);
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isEmpty(logicActualdataSourceMap)) {
+            return logicTableDataNodes;
         }
-        return result;
+
+        return replaceLogicDataNodeWithActualDataNode(logicTableDataNodes, logicActualdataSourceMap);
     }
     
-    private static Optional<DialectTableMetaDataLoader> findDialectTableMetaDataLoader(final SchemaBuilderMaterials materials) {
-        for (DialectTableMetaDataLoader each : ShardingSphereServiceLoader.getSingletonServiceInstances(DialectTableMetaDataLoader.class)) {
-            if (each.getDatabaseType().equals(materials.getDatabaseType().getName())) {
-                return Optional.of(each);
-            }
-        }
-        return Optional.empty();
+    private static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(
+            final List<DataNodeContainedRule> dataNodeContainedRuleList) {
+        BinaryOperator<Collection<DataNode>> logicTableMergeFunction = getMergeFunction(DataNode.class);
+        Map<String, Collection<DataNode>> logicTableDataNodes = dataNodeContainedRuleList.stream()
+                .flatMap(each -> each.getAllDataNodes().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, logicTableMergeFunction));
+        return logicTableDataNodes;
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
-        for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
-        }
-        for (Future<Map<String, TableMetaData>> each : futures) {
-            try {
-                tables.putAll(each.get());
-            } catch (final InterruptedException | ExecutionException ex) {
-                if (ex.getCause() instanceof SQLException) {
-                    throw (SQLException) ex.getCause();
-                }
-                throw new ShardingSphereException(ex);
-            }
-        }
+    private static Map<String, Collection<String>> getLogicActualDataSourceMap(final SchemaBuilderMaterials materials) {
+        BinaryOperator<Collection<String>> dataSourceContainedMergeFunction = getMergeFunction(String.class);
+        Map<String, Collection<String>> dataSourceContainedMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataSourceContainedRule)
+                .flatMap(each -> ((DataSourceContainedRule) each).getDataSourceMapper().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, dataSourceContainedMergeFunction));
+        return dataSourceContainedMap;
     }
     
-    private static void appendDefaultRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<String> existedTableNames = getExistedTables(materials.getRules(), tables);
-        for (Entry<String, DataSource> entry : materials.getDataSourceMap().entrySet()) {
-            Collection<String> tableNames = SchemaMetaDataLoader.loadAllTableNames(entry.getValue(), materials.getDatabaseType());
-            tableNames.removeAll(existedTableNames);
-            for (String each : tableNames) {
-                tables.put(each, loadTableMetaData(each, entry.getValue(), materials.getDatabaseType()));
-            }
-        }
+    private static <T> BinaryOperator<Collection<T>> getMergeFunction(final Class<T> tClass) {
+        return (a, b) -> {
+            Collection<T> result = (a instanceof LinkedList) ? a : new LinkedList<>(a);
+            result.addAll(b);
+            return result;
+        };
     }
     
-    private static TableMetaData loadTableMetaData(final String tableName, final DataSource dataSource, final DatabaseType databaseType) throws SQLException {
-        TableMetaData result = new TableMetaData(tableName);
-        try (Connection connection = new MetaDataLoaderConnectionAdapter(databaseType, dataSource.getConnection())) {
-            result.getColumns().putAll(loadColumnMetaDataMap(tableName, databaseType, connection));
+    private static Map<String, Collection<DataNode>> replaceLogicDataNodeWithActualDataNode(
+            final Map<String, Collection<DataNode>> logicTableDataNodes,
+            final Map<String, Collection<String>> logicActualdataSourceMap) {
+        Map<String, Collection<DataNode>> replaceResult = new HashMap<>(logicTableDataNodes.size(), 1);
+        for (Map.Entry<String, Collection<DataNode>> entry : logicTableDataNodes.entrySet()) {
+            Collection<DataNode> dataNodes = entry.getValue();
+            Collection<DataNode> actualDataNodeList = new LinkedList<>();
+            for (DataNode each : dataNodes) {
+                Collection<String> actualDataSourceNameList = logicActualdataSourceMap.get(each.getDataSourceName());
+                if (CollectionUtils.isEmpty(actualDataSourceNameList)) {
+                    actualDataNodeList.add(each);
+                } else {
+                    for (String actualDataSourceName : actualDataSourceNameList) {
+                        actualDataNodeList.add(new DataNode(actualDataSourceName, each.getTableName()));
+                    }
+                }
+            }
+            replaceResult.put(entry.getKey(), actualDataNodeList);
         }
-        return result;
+        return replaceResult;
     }
     
-    private static Map<String, ColumnMetaData> loadColumnMetaDataMap(final String tableName, final DatabaseType databaseType, final Connection connection) throws SQLException {
-        return ColumnMetaDataLoader.load(connection, tableName, databaseType).stream().collect(Collectors.toMap(ColumnMetaData::getName, each -> each, (a, b) -> b, LinkedHashMap::new));
-    }
-    
-    private static Collection<String> getExistedTables(final Collection<ShardingSphereRule> rules, final Map<String, TableMetaData> tables) {
-        Collection<String> result = new LinkedHashSet<>();
-        for (ShardingSphereRule each : rules) {
-            if (each instanceof DataNodeContainedRule) {
-                result.addAll(((DataNodeContainedRule) each).getAllActualTables());
+    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials,

Review comment:
       Why is this method changed? It looks the same as it used to be.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r672978287



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());

Review comment:
       Can this code be extracted into a method?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       @Beyondeclipse It is better to adjust the execution order of the code instead of adding the `toAppendLoadTables` parameter.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());
+            for (String table : toAppendLoadTables) {
+                futures.add(EXECUTOR_SERVICE.submit(() -> TableMetaDataBuilder.load(table, materials)));
+            }
+            for (Future<Optional<TableMetaData>> each : futures) {
+                try {
+                    each.get().map(optional -> result.put(optional.getName(), optional));
+                } catch (final InterruptedException | ExecutionException ex) {
+                    if (ex.getCause() instanceof SQLException) {
+                        throw (SQLException) ex.getCause();
                     }
+                    throw new ShardingSphereException(ex);
                 }
             }
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables,
+            final Collection<String> ruleLogicTables) throws SQLException {
         Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
+            appendDialectRemainTables(dialectLoader.get(), materials, tables, ruleLogicTables);
             return;
         }
         appendDefaultRemainTables(materials, tables);
     }
     
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {

Review comment:
       Why delete this method?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread

Review comment:
       Please delete extra comments.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       Why change it to `Collections.emptyList()`?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r674484489



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       I add a method without this parameter.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ec22d15) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.00%`.
   > The diff coverage is `32.39%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##             master   #11345    +/-   ##
   ==========================================
     Coverage     63.44%   63.45%            
   - Complexity     1028     1091    +63     
   ==========================================
     Files          2216     2221     +5     
     Lines         33753    33897   +144     
     Branches       5897     5907    +10     
   ==========================================
   + Hits          21416    21509    +93     
   - Misses        10629    10669    +40     
   - Partials       1708     1719    +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `83.33% <0.00%> (-13.45%)` | :arrow_down: |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `52.41% <34.84%> (-16.82%)` | :arrow_down: |
   | [...ra/route/engine/single/SingleTableRouteEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtcm91dGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL3JvdXRlL2VuZ2luZS9zaW5nbGUvU2luZ2xlVGFibGVSb3V0ZUVuZ2luZS5qYXZh) | `57.14% <0.00%> (-24.68%)` | :arrow_down: |
   | [.../postgresql/codec/PostgreSQLPacketCodecEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGIvcHJvdG9jb2wvcG9zdGdyZXNxbC9jb2RlYy9Qb3N0Z3JlU1FMUGFja2V0Q29kZWNFbmdpbmUuamF2YQ==) | `63.33% <0.00%> (-9.09%)` | :arrow_down: |
   | [...e/validator/ddl/ShardingDDLStatementValidator.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3ZhbGlkYXRvci9kZGwvU2hhcmRpbmdERExTdGF0ZW1lbnRWYWxpZGF0b3IuamF2YQ==) | `95.45% <0.00%> (-4.55%)` | :arrow_down: |
   | [.../route/engine/type/ShardingRouteEngineFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3R5cGUvU2hhcmRpbmdSb3V0ZUVuZ2luZUZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-4.26%)` | :arrow_down: |
   | [...ardingsphere/infra/route/SingleTableSQLRouter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtcm91dGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL3JvdXRlL1NpbmdsZVRhYmxlU1FMUm91dGVyLmphdmE=) | `13.63% <0.00%> (-3.04%)` | :arrow_down: |
   | [...che/shardingsphere/sharding/rule/ShardingRule.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcnVsZS9TaGFyZGluZ1J1bGUuamF2YQ==) | `74.01% <0.00%> (-2.73%)` | :arrow_down: |
   | [...broadcast/ShardingTableBroadcastRoutingEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3R5cGUvYnJvYWRjYXN0L1NoYXJkaW5nVGFibGVCcm9hZGNhc3RSb3V0aW5nRW5naW5lLmphdmE=) | `68.25% <0.00%> (-1.90%)` | :arrow_down: |
   | [...ardingsphere/infra/route/context/RouteContext.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtcm91dGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL3JvdXRlL2NvbnRleHQvUm91dGVDb250ZXh0LmphdmE=) | `94.11% <0.00%> (-0.89%)` | :arrow_down: |
   | ... and [93 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...ec22d15](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r675259189



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -129,16 +120,40 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         }
         return Optional.empty();
     }
-    
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+
+    private static Map<String, String> getActualTable2LogicTableMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
+        }
+
+        int size = dataNodeContainedRuleList.stream()
+                .mapToInt(each -> each.getAllDataNodes().entrySet().stream().mapToInt(entry -> entry.getValue().size()).sum())
+                .sum();
+        Map<String, String> actualTable2LogicTableMap = new HashMap<>(size);
+        for (DataNodeContainedRule each : dataNodeContainedRuleList) {
+            for (Entry<String, Collection<DataNode>> entry : each.getAllDataNodes().entrySet()) {
+                List<String> actualTables = entry.getValue().stream().map(DataNode::getTableName).collect(Collectors.toList());
+                for (String oneActualTalbe : actualTables) {
+                    actualTable2LogicTableMap.put(oneActualTalbe, entry.getKey());
+                }
+            }
+        }
+        return actualTable2LogicTableMap;
+    }
+
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials,
+            final Map<String, String> ruleActualTable2LogicTableMap, final Map<String, TableMetaData> tables) throws SQLException {
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each)));

Review comment:
       @Beyondeclipse Can you send your contact information to my email?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ea710e2) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.09%`.
   > The diff coverage is `55.55%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   + Coverage     63.44%   63.54%   +0.09%     
   - Complexity     1028     1061      +33     
   ============================================
     Files          2216     2223       +7     
     Lines         33753    33836      +83     
     Branches       5897     5903       +6     
   ============================================
   + Hits          21416    21502      +86     
   + Misses        10629    10620       -9     
   - Partials       1708     1714       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `68.53% <55.55%> (-0.70%)` | :arrow_down: |
   | [.../postgresql/codec/PostgreSQLPacketCodecEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGIvcHJvdG9jb2wvcG9zdGdyZXNxbC9jb2RlYy9Qb3N0Z3JlU1FMUGFja2V0Q29kZWNFbmdpbmUuamF2YQ==) | `63.33% <0.00%> (-9.09%)` | :arrow_down: |
   | [...ardingsphere/governance/core/GovernanceFacade.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9Hb3Zlcm5hbmNlRmFjYWRlLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `88.23% <0.00%> (ø)` | |
   | [...l/parser/core/database/visitor/SQLVisitorRule.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9jb3JlL2RhdGFiYXNlL3Zpc2l0b3IvU1FMVmlzaXRvclJ1bGUuamF2YQ==) | `100.00% <0.00%> (ø)` | |
   | [...zed/asserts/segment/orderby/OrderByItemAssert.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvYXNzZXJ0cy9zZWdtZW50L29yZGVyYnkvT3JkZXJCeUl0ZW1Bc3NlcnQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...t/dml/order/item/ExpressionOrderByItemSegment.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLXN0YXRlbWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9zcWwvY29tbW9uL3NlZ21lbnQvZG1sL29yZGVyL2l0ZW0vRXhwcmVzc2lvbk9yZGVyQnlJdGVtU2VnbWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...derby/item/impl/ExpectedExpressionOrderByItem.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvamF4Yi9jYXNlcy9kb21haW4vc2VnbWVudC9pbXBsL29yZGVyYnkvaXRlbS9pbXBsL0V4cGVjdGVkRXhwcmVzc2lvbk9yZGVyQnlJdGVtLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | [...l/parser/core/ShardingRuleSQLStatementVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctZGlzdHNxbC9zaGFyZGluZ3NwaGVyZS1zaGFyZGluZy1kaXN0c3FsLXBhcnNlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvZGlzdHNxbC9wYXJzZXIvY29yZS9TaGFyZGluZ1J1bGVTUUxTdGF0ZW1lbnRWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...ig/swapper/GovernanceConfigurationYamlSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS95YW1sL2NvbmZpZy9zd2FwcGVyL0dvdmVybmFuY2VDb25maWd1cmF0aW9uWWFtbFN3YXBwZXIuamF2YQ==) | | |
   | ... and [64 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...ea710e2](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r672978287



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());

Review comment:
       Can this code be extracted into a method?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       @Beyondeclipse It is better to adjust the execution order of the code instead of adding the `toAppendLoadTables` parameter.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());
+            for (String table : toAppendLoadTables) {
+                futures.add(EXECUTOR_SERVICE.submit(() -> TableMetaDataBuilder.load(table, materials)));
+            }
+            for (Future<Optional<TableMetaData>> each : futures) {
+                try {
+                    each.get().map(optional -> result.put(optional.getName(), optional));
+                } catch (final InterruptedException | ExecutionException ex) {
+                    if (ex.getCause() instanceof SQLException) {
+                        throw (SQLException) ex.getCause();
                     }
+                    throw new ShardingSphereException(ex);
                 }
             }
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables,
+            final Collection<String> ruleLogicTables) throws SQLException {
         Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
+            appendDialectRemainTables(dialectLoader.get(), materials, tables, ruleLogicTables);
             return;
         }
         appendDefaultRemainTables(materials, tables);
     }
     
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {

Review comment:
       Why delete this method?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread

Review comment:
       Please delete extra comments.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       Why change it to `Collections.emptyList()`?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a6f3aae) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.01%`.
   > The diff coverage is `31.42%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   + Coverage     63.44%   63.46%   +0.01%     
   - Complexity     1028     1077      +49     
   ============================================
     Files          2216     2223       +7     
     Lines         33753    33882     +129     
     Branches       5897     5910      +13     
   ============================================
   + Hits          21416    21504      +88     
   - Misses        10629    10664      +35     
   - Partials       1708     1714       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `83.33% <0.00%> (-13.45%)` | :arrow_down: |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `51.63% <33.84%> (-17.60%)` | :arrow_down: |
   | [.../postgresql/codec/PostgreSQLPacketCodecEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGIvcHJvdG9jb2wvcG9zdGdyZXNxbC9jb2RlYy9Qb3N0Z3JlU1FMUGFja2V0Q29kZWNFbmdpbmUuamF2YQ==) | `63.33% <0.00%> (-9.09%)` | :arrow_down: |
   | [...ardingsphere/governance/core/GovernanceFacade.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9Hb3Zlcm5hbmNlRmFjYWRlLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `88.23% <0.00%> (ø)` | |
   | [.../segment/select/projection/ProjectionsContext.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9Qcm9qZWN0aW9uc0NvbnRleHQuamF2YQ==) | `94.73% <0.00%> (ø)` | |
   | [...l/parser/core/database/visitor/SQLVisitorRule.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9jb3JlL2RhdGFiYXNlL3Zpc2l0b3IvU1FMVmlzaXRvclJ1bGUuamF2YQ==) | `100.00% <0.00%> (ø)` | |
   | [.../select/projection/impl/AggregationProjection.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9pbXBsL0FnZ3JlZ2F0aW9uUHJvamVjdGlvbi5qYXZh) | `100.00% <0.00%> (ø)` | |
   | [...zed/asserts/segment/orderby/OrderByItemAssert.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvYXNzZXJ0cy9zZWdtZW50L29yZGVyYnkvT3JkZXJCeUl0ZW1Bc3NlcnQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...d/asserts/segment/projection/ProjectionAssert.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvYXNzZXJ0cy9zZWdtZW50L3Byb2plY3Rpb24vUHJvamVjdGlvbkFzc2VydC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [73 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...a6f3aae](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7de9375) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **decrease** coverage by `0.15%`.
   > The diff coverage is `84.93%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   - Coverage     63.44%   63.29%   -0.16%     
   - Complexity     1028     1091      +63     
   ============================================
     Files          2216     2223       +7     
     Lines         33753    34032     +279     
     Branches       5897     5930      +33     
   ============================================
   + Hits          21416    21540     +124     
   - Misses        10629    10768     +139     
   - Partials       1708     1724      +16     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ilder/loader/dialect/MySQLTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9NeVNRTFRhYmxlTWV0YURhdGFMb2FkZXIuamF2YQ==) | `85.45% <0.00%> (-1.34%)` | :arrow_down: |
   | [...lder/loader/dialect/OracleTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9PcmFjbGVUYWJsZU1ldGFEYXRhTG9hZGVyLmphdmE=) | `84.93% <0.00%> (-1.18%)` | :arrow_down: |
   | [.../loader/dialect/PostgreSQLTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9Qb3N0Z3JlU1FMVGFibGVNZXRhRGF0YUxvYWRlci5qYXZh) | `84.37% <ø> (-1.34%)` | :arrow_down: |
   | [...r/loader/dialect/SQLServerTableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9TUUxTZXJ2ZXJUYWJsZU1ldGFEYXRhTG9hZGVyLmphdmE=) | `84.90% <0.00%> (-1.64%)` | :arrow_down: |
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `96.77% <ø> (ø)` | |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `88.88% <88.40%> (+19.65%)` | :arrow_up: |
   | [.../builder/loader/dialect/H2TableMetaDataLoader.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9sb2FkZXIvZGlhbGVjdC9IMlRhYmxlTWV0YURhdGFMb2FkZXIuamF2YQ==) | `87.20% <100.00%> (+0.15%)` | :arrow_up: |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `0.00% <0.00%> (-88.24%)` | :arrow_down: |
   | [...ere/agent/metrics/api/advice/DataSourceAdvice.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtYWdlbnQvc2hhcmRpbmdzcGhlcmUtYWdlbnQtcGx1Z2lucy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1wbHVnaW4tbWV0cmljcy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYWdlbnQvbWV0cmljcy9hcGkvYWR2aWNlL0RhdGFTb3VyY2VBZHZpY2UuamF2YQ==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [...onfig/subscriber/GlobalRuleRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9jb25maWcvc3Vic2NyaWJlci9HbG9iYWxSdWxlUmVnaXN0cnlTdWJzY3JpYmVyLmphdmE=) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | ... and [127 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...7de9375](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r677928958



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderWithDefaultLoader.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.infra.database.type.DatabaseType;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.ColumnMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.SchemaMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.adapter.MetaDataLoaderConnectionAdapter;
+import org.apache.shardingsphere.infra.metadata.schema.model.ColumnMetaData;
+import org.apache.shardingsphere.infra.metadata.schema.model.TableMetaData;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaBuilderWithDefaultLoader {

Review comment:
       @Beyondeclipse Please add final.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -76,109 +69,87 @@
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
-    
+
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
-        }
-        return result;
-    }
-    
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
+        Map<String, Collection<DataNode>> logicTable2DataNodes = getLogicTable2DataNodes(materials);
+
+        Optional<DialectTableMetaDataLoader> dialectLoader = SchemaBuilderWithDialectLoader.findDialectTableMetaDataLoader(materials);
+        Map<String, TableMetaData> result;
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
-            return;
-        }
-        appendDefaultRemainTables(materials, tables);
-    }
-    
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (tables.containsKey(table)) {
-                        TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
-                        result.put(table, metaData);
-                    }
-                }
-            }
+            result = SchemaBuilderWithDialectLoader.build(dialectLoader.get(), EXECUTOR_SERVICE, materials, logicTable2DataNodes);
+        } else {
+            result = SchemaBuilderWithDefaultLoader.build(EXECUTOR_SERVICE, materials, logicTable2DataNodes);
         }
         return result;
     }
-    
-    private static Optional<DialectTableMetaDataLoader> findDialectTableMetaDataLoader(final SchemaBuilderMaterials materials) {
-        for (DialectTableMetaDataLoader each : ShardingSphereServiceLoader.getSingletonServiceInstances(DialectTableMetaDataLoader.class)) {
-            if (each.getDatabaseType().equals(materials.getDatabaseType().getName())) {
-                return Optional.of(each);
-            }
+
+    private static Map<String, Collection<DataNode>> getLogicTable2DataNodes(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
         }
-        return Optional.empty();
-    }
-    
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
-        for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+        Map<String, Collection<DataNode>> logicTable2DataNodes = dataNodeContainedRuleList.stream()
+                .flatMap(each -> each.getAllDataNodes().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> {
+                    Collection<DataNode> result = new LinkedList<>(a);
+                    result.addAll(b);
+                    return result;
+                }));
+
+        Map<String, Collection<String>> dataSourceContainedMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataSourceContainedRule)
+                .flatMap(each -> ((DataSourceContainedRule) each).getDataSourceMapper().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> {
+                    Collection<String> result = new ArrayList<>(a.size() + b.size());
+                    result.addAll(a);
+                    result.addAll(b);
+                    return result;
+                }));
+        if (MapUtils.isEmpty(dataSourceContainedMap)) {
+            return logicTable2DataNodes;
         }
-        for (Future<Map<String, TableMetaData>> each : futures) {
-            try {
-                tables.putAll(each.get());
-            } catch (final InterruptedException | ExecutionException ex) {
-                if (ex.getCause() instanceof SQLException) {
-                    throw (SQLException) ex.getCause();
+
+        Map<String, Collection<DataNode>> replaceResult = new HashMap<>(logicTable2DataNodes.size(), 1);
+        for (Map.Entry<String, Collection<DataNode>> entry : logicTable2DataNodes.entrySet()) {
+            Collection<DataNode> dataNodes = entry.getValue();
+            Collection<DataNode> newDataNodeList = new LinkedList<>();
+            for (DataNode each : dataNodes) {
+                Collection<String> toReplaceDataSourceNames = dataSourceContainedMap.get(each.getDataSourceName());
+                if (CollectionUtils.isEmpty(toReplaceDataSourceNames)) {
+                    newDataNodeList.add(each);
+                } else {
+                    for (String newDataSourceName : toReplaceDataSourceNames) {
+                        newDataNodeList.add(new DataNode(newDataSourceName, each.getTableName()));

Review comment:
       @Beyondeclipse Why we need to replace dataSource name?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderWithDialectLoader.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.infra.metadata.schema.builder.spi.DialectTableMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.model.TableMetaData;
+import org.apache.shardingsphere.infra.spi.ShardingSphereServiceLoader;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaBuilderWithDialectLoader {

Review comment:
       @Beyondeclipse Please add final.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -76,109 +69,87 @@
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
-    
+
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
-        }
-        return result;
-    }
-    
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
+        Map<String, Collection<DataNode>> logicTable2DataNodes = getLogicTable2DataNodes(materials);
+
+        Optional<DialectTableMetaDataLoader> dialectLoader = SchemaBuilderWithDialectLoader.findDialectTableMetaDataLoader(materials);
+        Map<String, TableMetaData> result;
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
-            return;
-        }
-        appendDefaultRemainTables(materials, tables);
-    }
-    
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (tables.containsKey(table)) {
-                        TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
-                        result.put(table, metaData);
-                    }
-                }
-            }
+            result = SchemaBuilderWithDialectLoader.build(dialectLoader.get(), EXECUTOR_SERVICE, materials, logicTable2DataNodes);
+        } else {
+            result = SchemaBuilderWithDefaultLoader.build(EXECUTOR_SERVICE, materials, logicTable2DataNodes);
         }
         return result;
     }
-    
-    private static Optional<DialectTableMetaDataLoader> findDialectTableMetaDataLoader(final SchemaBuilderMaterials materials) {
-        for (DialectTableMetaDataLoader each : ShardingSphereServiceLoader.getSingletonServiceInstances(DialectTableMetaDataLoader.class)) {
-            if (each.getDatabaseType().equals(materials.getDatabaseType().getName())) {
-                return Optional.of(each);
-            }
+
+    private static Map<String, Collection<DataNode>> getLogicTable2DataNodes(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
         }
-        return Optional.empty();
-    }
-    
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
-        for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+        Map<String, Collection<DataNode>> logicTable2DataNodes = dataNodeContainedRuleList.stream()

Review comment:
       @Beyondeclipse Can we extract this two param `logicTable2DataNodes`, `dataSourceContainedMap` to a private method?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -76,109 +69,87 @@
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
-    
+
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
-        }
-        return result;
-    }
-    
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
+        Map<String, Collection<DataNode>> logicTable2DataNodes = getLogicTable2DataNodes(materials);
+

Review comment:
       @Beyondeclipse Remove useless black line.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderWithDefaultLoader.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.infra.database.type.DatabaseType;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.ColumnMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.SchemaMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.adapter.MetaDataLoaderConnectionAdapter;
+import org.apache.shardingsphere.infra.metadata.schema.model.ColumnMetaData;
+import org.apache.shardingsphere.infra.metadata.schema.model.TableMetaData;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class SchemaBuilderWithDefaultLoader {
+
+    /**
+     * build table meta data with default loader.

Review comment:
       @Beyondeclipse Capitalize the first letter in java doc.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tristaZero commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

tristaZero commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-882014559


   @Beyondeclipse Thanks for your PR and test. We are working on the improvement of metadata loading.
   
   @totalo  @strongduanmu Could you give a review at this PR?


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r673020337



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       Because 'dialectLoader.load' is more efficient and I want it to load all the tables. After that , I can simply change the sharding-tables to logic-tables by 'actualTable2LogicTableMap' without connecting db.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());
+            for (String table : toAppendLoadTables) {
+                futures.add(EXECUTOR_SERVICE.submit(() -> TableMetaDataBuilder.load(table, materials)));
+            }
+            for (Future<Optional<TableMetaData>> each : futures) {
+                try {
+                    each.get().map(optional -> result.put(optional.getName(), optional));
+                } catch (final InterruptedException | ExecutionException ex) {
+                    if (ex.getCause() instanceof SQLException) {
+                        throw (SQLException) ex.getCause();
                     }
+                    throw new ShardingSphereException(ex);
                 }
             }
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables,
+            final Collection<String> ruleLogicTables) throws SQLException {
         Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
+            appendDialectRemainTables(dialectLoader.get(), materials, tables, ruleLogicTables);
             return;
         }
         appendDefaultRemainTables(materials, tables);
     }
     
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {

Review comment:
       I just moved this method to the bottom, since it was in the methods belong to 'buildActualTableMetaDataMap'.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread

Review comment:
       OK

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       I will extract these code to method 'appendRemainRuleLogicTables', and ruleLogicTables is useful for exclude loaded tables.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());

Review comment:
       OK.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-883314839


   > @Beyondeclipse Please add test case for your change.
   I will do it later.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter edited a comment on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter edited a comment on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a6f3aae) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.01%`.
   > The diff coverage is `31.42%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   + Coverage     63.44%   63.46%   +0.01%     
   - Complexity     1028     1077      +49     
   ============================================
     Files          2216     2223       +7     
     Lines         33753    33882     +129     
     Branches       5897     5910      +13     
   ============================================
   + Hits          21416    21504      +88     
   - Misses        10629    10664      +35     
   - Partials       1708     1714       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ere/infra/metadata/schema/model/TableMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbW9kZWwvVGFibGVNZXRhRGF0YS5qYXZh) | `83.33% <0.00%> (-13.45%)` | :arrow_down: |
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `51.63% <33.84%> (-17.60%)` | :arrow_down: |
   | [.../postgresql/codec/PostgreSQLPacketCodecEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wvc2hhcmRpbmdzcGhlcmUtZGItcHJvdG9jb2wtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGIvcHJvdG9jb2wvcG9zdGdyZXNxbC9jb2RlYy9Qb3N0Z3JlU1FMUGFja2V0Q29kZWNFbmdpbmUuamF2YQ==) | `63.33% <0.00%> (-9.09%)` | :arrow_down: |
   | [...ardingsphere/governance/core/GovernanceFacade.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9Hb3Zlcm5hbmNlRmFjYWRlLmphdmE=) | `100.00% <0.00%> (ø)` | |
   | [...phere/governance/core/registry/RegistryCenter.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9SZWdpc3RyeUNlbnRlci5qYXZh) | `88.23% <0.00%> (ø)` | |
   | [.../segment/select/projection/ProjectionsContext.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9Qcm9qZWN0aW9uc0NvbnRleHQuamF2YQ==) | `94.73% <0.00%> (ø)` | |
   | [...l/parser/core/database/visitor/SQLVisitorRule.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtc3FsLXBhcnNlci9zaGFyZGluZ3NwaGVyZS1zcWwtcGFyc2VyLWVuZ2luZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc3FsL3BhcnNlci9jb3JlL2RhdGFiYXNlL3Zpc2l0b3IvU1FMVmlzaXRvclJ1bGUuamF2YQ==) | `100.00% <0.00%> (ø)` | |
   | [.../select/projection/impl/AggregationProjection.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9pbXBsL0FnZ3JlZ2F0aW9uUHJvamVjdGlvbi5qYXZh) | `100.00% <0.00%> (ø)` | |
   | [...zed/asserts/segment/orderby/OrderByItemAssert.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvYXNzZXJ0cy9zZWdtZW50L29yZGVyYnkvT3JkZXJCeUl0ZW1Bc3NlcnQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...d/asserts/segment/projection/ProjectionAssert.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtdGVzdC9zaGFyZGluZ3NwaGVyZS1wYXJzZXItdGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvdGVzdC9zcWwvcGFyc2VyL3BhcmFtZXRlcml6ZWQvYXNzZXJ0cy9zZWdtZW50L3Byb2plY3Rpb24vUHJvamVjdGlvbkFzc2VydC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [73 more](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...a6f3aae](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-882140062


   @Beyondeclipse Can you take a look at the ci exception first? There are failed test cases in the integration test.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r674482739



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -147,9 +173,35 @@ private static void appendDialectRemainTables(final DialectTableMetaDataLoader d
             }
         }
     }
-    
+
+    private static Map<String, String> getActualTable2LogicTableMap(final SchemaBuilderMaterials materials, final Collection<String> ruleLogicTables) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
+        }
+
+        int size = dataNodeContainedRuleList.stream()
+                .mapToInt(each -> each.getAllDataNodes().entrySet().stream()
+                        .mapToInt(entry -> entry.getValue().size())
+                        .sum())
+                .sum();
+        Map<String, String> actualTable2LogicTableMap = new HashMap<>(size);

Review comment:
       Do you mean that change 'entry' to 'each'? But it is in a map.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +141,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));
         }
         for (Future<Map<String, TableMetaData>> each : futures) {
             try {
-                tables.putAll(each.get());
+                Map<String, TableMetaData> tableMetaMap = each.get();
+                for (Map.Entry<String, TableMetaData> one : tableMetaMap.entrySet()) {
+                    String actualTableName = one.getKey();
+                    String logicTableName = actualTable2LogicTableMap.get(actualTableName);

Review comment:
       OK

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +141,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));
         }
         for (Future<Map<String, TableMetaData>> each : futures) {
             try {
-                tables.putAll(each.get());
+                Map<String, TableMetaData> tableMetaMap = each.get();
+                for (Map.Entry<String, TableMetaData> one : tableMetaMap.entrySet()) {

Review comment:
       OK.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tuichenchuxin commented on a change in pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

tuichenchuxin commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r683920977



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderWithDefaultLoader.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.infra.database.type.DatabaseType;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.ColumnMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.SchemaMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.builder.loader.adapter.MetaDataLoaderConnectionAdapter;
+import org.apache.shardingsphere.infra.metadata.schema.model.ColumnMetaData;
+import org.apache.shardingsphere.infra.metadata.schema.model.TableMetaData;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderWithDefaultLoader {
+    
+    /**
+     * Build table metadata with default loader.
+     * @param executorService executorService
+     * @param materials schema builder materials
+     * @param logicTableDataNodesMap map of logicTable to DataNodes
+     * @return metadata map
+     * @throws SQLException SQL exception
+     */
+    public static Map<String, TableMetaData> build(final ExecutorService executorService, final SchemaBuilderMaterials materials,
+            final Map<String, Collection<DataNode>> logicTableDataNodesMap) throws SQLException {
+        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
+        result.putAll(appendActualTables(executorService, materials, logicTableDataNodesMap));
+        result.putAll(appendRuleLogicTables(executorService, materials, logicTableDataNodesMap));
+        return result;
+    }
+    
+    private static Map<String, TableMetaData> appendActualTables(final ExecutorService executorService, final SchemaBuilderMaterials materials,
+            final Map<String, Collection<DataNode>> logicTableDataNodesMap) throws SQLException {
+        Collection<String> existedTableNames = logicTableDataNodesMap.values().stream()
+                .flatMap(each -> each.stream().map(DataNode::getTableName))
+                .collect(Collectors.toSet());
+
+        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size());
+        for (Map.Entry<String, DataSource> entry : materials.getDataSourceMap().entrySet()) {
+            Collection<String> tableNames = SchemaMetaDataLoader.loadAllTableNames(entry.getValue(), materials.getDatabaseType());
+            tableNames.removeAll(existedTableNames);
+
+            Collection<Future<TableMetaData>> futures = new ArrayList<>(tableNames.size());
+            for (String each : tableNames) {
+                futures.add(executorService.submit(() -> loadTableMetaData(each, entry.getValue(), materials.getDatabaseType())));

Review comment:
       Each table is individually submitted to the thread pool, can it really improve performance?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -66,119 +59,114 @@
     }
     
     /**
-     * build actual and logic table meta data.
+     * Build actual and logic table metadata.
      *
      * @param materials schema builder materials
-     * @return actual and logic table meta data
+     * @return actual and logic table metadata
      * @throws SQLException SQL exception
      */
     public static Map<TableMetaData, TableMetaData> build(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = getLogicTableDataNodesMap(materials);
+        Optional<DialectTableMetaDataLoader> dialectLoader = SchemaBuilderWithDialectLoader.findDialectTableMetaDataLoader(materials);
+        Map<String, TableMetaData> result;
+        if (dialectLoader.isPresent()) {
+            result = SchemaBuilderWithDialectLoader.build(dialectLoader.get(), EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
+        } else {
+            result = SchemaBuilderWithDefaultLoader.build(EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
-        if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
-            return;
+    private static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
         }
-        appendDefaultRemainTables(materials, tables);
-    }
-    
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (tables.containsKey(table)) {
-                        TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
-                        result.put(table, metaData);
-                    }
-                }
-            }
+
+        Map<String, Collection<DataNode>> logicTableDataNodes = getLogicTableDataNodesMap(dataNodeContainedRuleList);
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isEmpty(logicActualdataSourceMap)) {
+            return logicTableDataNodes;
         }
-        return result;
+
+        return replaceLogicDataNodeWithActualDataNode(logicTableDataNodes, logicActualdataSourceMap);
     }
     
-    private static Optional<DialectTableMetaDataLoader> findDialectTableMetaDataLoader(final SchemaBuilderMaterials materials) {
-        for (DialectTableMetaDataLoader each : ShardingSphereServiceLoader.getSingletonServiceInstances(DialectTableMetaDataLoader.class)) {
-            if (each.getDatabaseType().equals(materials.getDatabaseType().getName())) {
-                return Optional.of(each);
-            }
-        }
-        return Optional.empty();
+    private static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(
+            final List<DataNodeContainedRule> dataNodeContainedRuleList) {
+        BinaryOperator<Collection<DataNode>> logicTableMergeFunction = getMergeFunction(DataNode.class);
+        Map<String, Collection<DataNode>> logicTableDataNodes = dataNodeContainedRuleList.stream()
+                .flatMap(each -> each.getAllDataNodes().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, logicTableMergeFunction));
+        return logicTableDataNodes;
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
-        for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
-        }
-        for (Future<Map<String, TableMetaData>> each : futures) {
-            try {
-                tables.putAll(each.get());
-            } catch (final InterruptedException | ExecutionException ex) {
-                if (ex.getCause() instanceof SQLException) {
-                    throw (SQLException) ex.getCause();
-                }
-                throw new ShardingSphereException(ex);
-            }
-        }
+    private static Map<String, Collection<String>> getLogicActualDataSourceMap(final SchemaBuilderMaterials materials) {
+        BinaryOperator<Collection<String>> dataSourceContainedMergeFunction = getMergeFunction(String.class);
+        Map<String, Collection<String>> dataSourceContainedMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataSourceContainedRule)
+                .flatMap(each -> ((DataSourceContainedRule) each).getDataSourceMapper().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, dataSourceContainedMergeFunction));
+        return dataSourceContainedMap;
     }
     
-    private static void appendDefaultRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Collection<String> existedTableNames = getExistedTables(materials.getRules(), tables);
-        for (Entry<String, DataSource> entry : materials.getDataSourceMap().entrySet()) {
-            Collection<String> tableNames = SchemaMetaDataLoader.loadAllTableNames(entry.getValue(), materials.getDatabaseType());
-            tableNames.removeAll(existedTableNames);
-            for (String each : tableNames) {
-                tables.put(each, loadTableMetaData(each, entry.getValue(), materials.getDatabaseType()));
-            }
-        }
+    private static <T> BinaryOperator<Collection<T>> getMergeFunction(final Class<T> tClass) {

Review comment:
       `tClass` is not used.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -66,119 +59,114 @@
     }
     
     /**
-     * build actual and logic table meta data.
+     * Build actual and logic table metadata.
      *
      * @param materials schema builder materials
-     * @return actual and logic table meta data
+     * @return actual and logic table metadata
      * @throws SQLException SQL exception
      */
     public static Map<TableMetaData, TableMetaData> build(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
-                    }
-                }
-            }
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = getLogicTableDataNodesMap(materials);
+        Optional<DialectTableMetaDataLoader> dialectLoader = SchemaBuilderWithDialectLoader.findDialectTableMetaDataLoader(materials);
+        Map<String, TableMetaData> result;
+        if (dialectLoader.isPresent()) {
+            result = SchemaBuilderWithDialectLoader.build(dialectLoader.get(), EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
+        } else {
+            result = SchemaBuilderWithDefaultLoader.build(EXECUTOR_SERVICE, materials, logicTableDataNodesMap);
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
-        Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
-        if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
-            return;
+    private static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
         }
-        appendDefaultRemainTables(materials, tables);
-    }
-    
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) {
-        Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (tables.containsKey(table)) {
-                        TableMetaData metaData = TableMetaDataBuilder.decorate(table, tables.get(table), materials.getRules());
-                        result.put(table, metaData);
-                    }
-                }
-            }
+
+        Map<String, Collection<DataNode>> logicTableDataNodes = getLogicTableDataNodesMap(dataNodeContainedRuleList);
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isEmpty(logicActualdataSourceMap)) {
+            return logicTableDataNodes;
         }
-        return result;
+
+        return replaceLogicDataNodeWithActualDataNode(logicTableDataNodes, logicActualdataSourceMap);

Review comment:
       I don’t seem to see this logic in the original code. What is the meaning of this logic?

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderWithDialectLoader.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.exception.ShardingSphereException;
+import org.apache.shardingsphere.infra.metadata.schema.builder.spi.DialectTableMetaDataLoader;
+import org.apache.shardingsphere.infra.metadata.schema.model.TableMetaData;
+import org.apache.shardingsphere.infra.spi.ShardingSphereServiceLoader;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderWithDialectLoader {
+    

Review comment:
       What is the point of extracting dialect loading and default loading separately?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] codecov-commenter commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

codecov-commenter commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-880562307


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11345](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3b77575) into [master](https://codecov.io/gh/apache/shardingsphere/commit/6bfe8a17ef75ae090aac2b456f2c460c6383ecf0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6bfe8a1) will **increase** coverage by `0.01%`.
   > The diff coverage is `58.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11345/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11345      +/-   ##
   ============================================
   + Coverage     63.44%   63.46%   +0.01%     
     Complexity     1028     1028              
   ============================================
     Files          2216     2216              
     Lines         33753    33762       +9     
     Branches       5897     5897              
   ============================================
   + Hits          21416    21426      +10     
   - Misses        10629    10631       +2     
   + Partials       1708     1705       -3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...e/infra/metadata/schema/builder/SchemaBuilder.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvYnVpbGRlci9TY2hlbWFCdWlsZGVyLmphdmE=) | `69.31% <58.82%> (+0.08%)` | :arrow_up: |
   | [...ct/projection/engine/ProjectionsContextEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9lbmdpbmUvUHJvamVjdGlvbnNDb250ZXh0RW5naW5lLmphdmE=) | `67.56% <0.00%> (+3.56%)` | :arrow_up: |
   | [...nt/select/projection/impl/ShorthandProjection.java](https://codecov.io/gh/apache/shardingsphere/pull/11345/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtYmluZGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9iaW5kZXIvc2VnbWVudC9zZWxlY3QvcHJvamVjdGlvbi9pbXBsL1Nob3J0aGFuZFByb2plY3Rpb24uamF2YQ==) | `100.00% <0.00%> (+25.00%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6bfe8a1...3b77575](https://codecov.io/gh/apache/shardingsphere/pull/11345?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-894078346


   @tuichenchuxin, @strongduanmu, @tristaZero , as your advice, I split this improve into few PRs, and this time it is only focus on the load optimization logic.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: Improve loading TableMetaData for DialectTableMetaDataLoader

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r684068425



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -148,9 +155,27 @@ private static void appendDialectRemainTables(final DialectTableMetaDataLoader d
         }
     }
     
+    private static void putTablesForFutures(final Map<String, TableMetaData> tableMetaMap, final Map<String, DataNode> logicTableFirstDataNodeMap,
+            final Map<String, TableMetaData> tables) {
+        Map<String, String> firstActualLogicTableMap = logicTableFirstDataNodeMap.entrySet().stream()
+                .collect(Collectors.toMap(entry -> entry.getValue().getTableName(), Map.Entry::getKey));
+        for (Map.Entry<String, TableMetaData> entry : tableMetaMap.entrySet()) {

Review comment:
       Same problem.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -76,7 +77,7 @@
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {

Review comment:
       @Beyondeclipse Please use static import for Entry.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {
+    
+    /**
+     * Get mapping between logic-table name and data nodes.
+     * @param materials schema builder materials

Review comment:
       @Beyondeclipse Please keep a blank line.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {

Review comment:
       @Beyondeclipse Add javadoc for this class.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -131,14 +132,20 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
     }
     
     private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = SchemaBuilderHelper.getLogicTableDataNodesMap(materials);
+        Map<String, DataNode> logicTableFirstDataNodeMap = SchemaBuilderHelper.getLogicTableFirstDataNodeMap(logicTableDataNodesMap);
+        
+        Map<String, Collection<String>> datasourceExcludeTablesMap = SchemaBuilderHelper.getDatasourceExcludeTablesMap(logicTableDataNodesMap, logicTableFirstDataNodeMap);

Review comment:
       @Beyondeclipse dataSourceExcludeTablesMap is better.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -148,9 +155,27 @@ private static void appendDialectRemainTables(final DialectTableMetaDataLoader d
         }
     }
     
+    private static void putTablesForFutures(final Map<String, TableMetaData> tableMetaMap, final Map<String, DataNode> logicTableFirstDataNodeMap,
+            final Map<String, TableMetaData> tables) {
+        Map<String, String> firstActualLogicTableMap = logicTableFirstDataNodeMap.entrySet().stream()
+                .collect(Collectors.toMap(entry -> entry.getValue().getTableName(), Map.Entry::getKey));
+        for (Map.Entry<String, TableMetaData> entry : tableMetaMap.entrySet()) {
+            String actualTableName = entry.getKey();
+            String logicTableName = firstActualLogicTableMap.get(actualTableName);
+            if (null != logicTableName && !logicTableName.equals(actualTableName)) {
+                if (!tables.containsKey(logicTableName)) {
+                    TableMetaData copyTableMetaData = entry.getValue();
+                    tables.put(logicTableName, new TableMetaData(logicTableName, copyTableMetaData.getColumns().values(), copyTableMetaData.getIndexes().values()));
+                }
+            } else {
+                tables.put(actualTableName, entry.getValue());
+            }
+        }
+    }
+    
     private static void appendDefaultRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
         Collection<String> existedTableNames = getExistedTables(materials.getRules(), tables);
-        for (Entry<String, DataSource> entry : materials.getDataSourceMap().entrySet()) {
+        for (Map.Entry<String, DataSource> entry : materials.getDataSourceMap().entrySet()) {

Review comment:
       Same problem.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {
+    
+    /**
+     * Get mapping between logic-table name and data nodes.
+     * @param materials schema builder materials
+     * @return logic-table to data nodes map
+     */
+    public static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .flatMap(each -> ((DataNodeContainedRule) each).getAllDataNodes().entrySet().stream())
+                .filter(entry -> entry.getValue().size() > 1)
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Review comment:
       Same problem.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -148,9 +155,27 @@ private static void appendDialectRemainTables(final DialectTableMetaDataLoader d
         }
     }
     
+    private static void putTablesForFutures(final Map<String, TableMetaData> tableMetaMap, final Map<String, DataNode> logicTableFirstDataNodeMap,
+            final Map<String, TableMetaData> tables) {
+        Map<String, String> firstActualLogicTableMap = logicTableFirstDataNodeMap.entrySet().stream()
+                .collect(Collectors.toMap(entry -> entry.getValue().getTableName(), Map.Entry::getKey));

Review comment:
       @Beyondeclipse Please add mergeFunction param for `Collectors.toMap()`.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {
+    
+    /**
+     * Get mapping between logic-table name and data nodes.
+     * @param materials schema builder materials
+     * @return logic-table to data nodes map
+     */
+    public static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .flatMap(each -> ((DataNodeContainedRule) each).getAllDataNodes().entrySet().stream())
+                .filter(entry -> entry.getValue().size() > 1)
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        if (MapUtils.isEmpty(logicTableDataNodesMap)) {
+            return Collections.emptyMap();
+        }
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isNotEmpty(logicActualdataSourceMap)) {
+            logicTableDataNodesMap = replaceLogicDataNodeWithActualDataNode(logicTableDataNodesMap, logicActualdataSourceMap);
+        }
+        return logicTableDataNodesMap;
+    }
+    
+    private static Map<String, Collection<String>> getLogicActualDataSourceMap(final SchemaBuilderMaterials materials) {
+        Map<String, Collection<String>> dataSourceContainedMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataSourceContainedRule)
+                .flatMap(each -> ((DataSourceContainedRule) each).getDataSourceMapper().entrySet().stream())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> {
+                    Collection<String> result = (a instanceof LinkedList) ? a : new LinkedList<>(a);
+                    result.addAll(b);
+                    return result;
+                }));
+        return dataSourceContainedMap;
+    }
+    
+    private static Map<String, Collection<DataNode>> replaceLogicDataNodeWithActualDataNode(
+            final Map<String, Collection<DataNode>> logicTableDataNodesMap,
+            final Map<String, Collection<String>> logicActualdataSourceMap) {
+        Map<String, Collection<DataNode>> replaceResult = new HashMap<>(logicTableDataNodesMap.size(), 1);
+        for (Map.Entry<String, Collection<DataNode>> entry : logicTableDataNodesMap.entrySet()) {
+            Collection<DataNode> dataNodes = entry.getValue();
+            Collection<DataNode> actualDataNodeList = new LinkedList<>();
+            for (DataNode each : dataNodes) {
+                Collection<String> actualDataSourceNameList = logicActualdataSourceMap.get(each.getDataSourceName());
+                if (CollectionUtils.isEmpty(actualDataSourceNameList)) {
+                    actualDataNodeList.add(each);
+                } else {
+                    for (String actualDataSourceName : actualDataSourceNameList) {
+                        actualDataNodeList.add(new DataNode(actualDataSourceName, each.getTableName()));
+                    }
+                }
+            }
+            replaceResult.put(entry.getKey(), actualDataNodeList);
+        }
+        return replaceResult;
+    }
+    
+    /**
+     * Get mapping between logic-table name and the first data node of 'logicTableDataNodesMap'.
+     * @param logicTableDataNodesMap data nodes map
+     * @return logic-table to first data node map
+     */
+    public static Map<String, DataNode> getLogicTableFirstDataNodeMap(final Map<String, Collection<DataNode>> logicTableDataNodesMap) {

Review comment:
       @Beyondeclipse Please add test case for public method.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {
+    
+    /**
+     * Get mapping between logic-table name and data nodes.
+     * @param materials schema builder materials
+     * @return logic-table to data nodes map
+     */
+    public static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .flatMap(each -> ((DataNodeContainedRule) each).getAllDataNodes().entrySet().stream())
+                .filter(entry -> entry.getValue().size() > 1)
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        if (MapUtils.isEmpty(logicTableDataNodesMap)) {
+            return Collections.emptyMap();
+        }
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isNotEmpty(logicActualdataSourceMap)) {

Review comment:
       !logicActualdataSourceMap.isEmpty() is better.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilderHelper.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.metadata.schema.builder;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.MapUtils;
+import org.apache.shardingsphere.infra.datanode.DataNode;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataNodeContainedRule;
+import org.apache.shardingsphere.infra.rule.identifier.type.DataSourceContainedRule;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public final class SchemaBuilderHelper {
+    
+    /**
+     * Get mapping between logic-table name and data nodes.
+     * @param materials schema builder materials
+     * @return logic-table to data nodes map
+     */
+    public static Map<String, Collection<DataNode>> getLogicTableDataNodesMap(final SchemaBuilderMaterials materials) {
+        Map<String, Collection<DataNode>> logicTableDataNodesMap = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .flatMap(each -> ((DataNodeContainedRule) each).getAllDataNodes().entrySet().stream())
+                .filter(entry -> entry.getValue().size() > 1)
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        if (MapUtils.isEmpty(logicTableDataNodesMap)) {
+            return Collections.emptyMap();
+        }
+        Map<String, Collection<String>> logicActualdataSourceMap = getLogicActualDataSourceMap(materials);
+        if (MapUtils.isNotEmpty(logicActualdataSourceMap)) {
+            logicTableDataNodesMap = replaceLogicDataNodeWithActualDataNode(logicTableDataNodesMap, logicActualdataSourceMap);
+        }
+        return logicTableDataNodesMap;

Review comment:
       Please change it to result.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-883264104


   @Beyondeclipse Please add test case for your 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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] totalo commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

totalo commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-884842296


   I'm sorry I'm late. Everything seems all right.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r674815965



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -129,16 +120,40 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         }
         return Optional.empty();
     }
-    
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+
+    private static Map<String, String> getActualTable2LogicTableMap(final SchemaBuilderMaterials materials) {
+        List<DataNodeContainedRule> dataNodeContainedRuleList = materials.getRules().stream()
+                .filter(each -> each instanceof DataNodeContainedRule)
+                .map(each -> (DataNodeContainedRule) each)
+                .collect(Collectors.toList());
+        if (CollectionUtils.isEmpty(dataNodeContainedRuleList)) {
+            return Collections.emptyMap();
+        }
+
+        int size = dataNodeContainedRuleList.stream()
+                .mapToInt(each -> each.getAllDataNodes().entrySet().stream().mapToInt(entry -> entry.getValue().size()).sum())
+                .sum();
+        Map<String, String> actualTable2LogicTableMap = new HashMap<>(size);
+        for (DataNodeContainedRule each : dataNodeContainedRuleList) {
+            for (Entry<String, Collection<DataNode>> entry : each.getAllDataNodes().entrySet()) {
+                List<String> actualTables = entry.getValue().stream().map(DataNode::getTableName).collect(Collectors.toList());
+                for (String oneActualTalbe : actualTables) {
+                    actualTable2LogicTableMap.put(oneActualTalbe, entry.getKey());
+                }
+            }
+        }
+        return actualTable2LogicTableMap;
+    }
+
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials,
+            final Map<String, String> ruleActualTable2LogicTableMap, final Map<String, TableMetaData> tables) throws SQLException {
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each)));

Review comment:
       @strongduanmu The original code also loaded all the tables, but use single-thread instand. And in my environment, this change reduce the loading time from 5m  to 50s with 400+ databases each has 50+ tables.
   In fact, the performance issue of original code is opening many times connections for the same database. And this change is try to improve it by reducing connections.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r673020337



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -130,15 +137,30 @@ private static void appendRemainTables(final SchemaBuilderMaterials materials, f
         return Optional.empty();
     }
     
-    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendDialectRemainTables(final DialectTableMetaDataLoader dialectLoader,
+            final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables, final Collection<String> ruleLogicTables) throws SQLException {
+
+        Map<String, String> actualTable2LogicTableMap = getActualTable2LogicTableMap(materials, ruleLogicTables);
+
         Collection<Future<Map<String, TableMetaData>>> futures = new LinkedList<>();
-        Collection<String> existedTables = getExistedTables(materials.getRules(), tables);
         for (DataSource each : materials.getDataSourceMap().values()) {
-            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, existedTables)));
+            futures.add(EXECUTOR_SERVICE.submit(() -> dialectLoader.load(each, Collections.emptyList())));

Review comment:
       Because 'dialectLoader.load' is more efficient and I want it to load all the tables. After that , I can simply change the sharding-tables to logic-tables by 'actualTable2LogicTableMap' without connecting db.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());
+            for (String table : toAppendLoadTables) {
+                futures.add(EXECUTOR_SERVICE.submit(() -> TableMetaDataBuilder.load(table, materials)));
+            }
+            for (Future<Optional<TableMetaData>> each : futures) {
+                try {
+                    each.get().map(optional -> result.put(optional.getName(), optional));
+                } catch (final InterruptedException | ExecutionException ex) {
+                    if (ex.getCause() instanceof SQLException) {
+                        throw (SQLException) ex.getCause();
                     }
+                    throw new ShardingSphereException(ex);
                 }
             }
         }
         return result;
     }
     
-    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {
+    private static void appendRemainTables(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables,
+            final Collection<String> ruleLogicTables) throws SQLException {
         Optional<DialectTableMetaDataLoader> dialectLoader = findDialectTableMetaDataLoader(materials);
         if (dialectLoader.isPresent()) {
-            appendDialectRemainTables(dialectLoader.get(), materials, tables);
+            appendDialectRemainTables(dialectLoader.get(), materials, tables, ruleLogicTables);
             return;
         }
         appendDefaultRemainTables(materials, tables);
     }
     
-    private static Map<String, TableMetaData> buildLogicTableMetaDataMap(final SchemaBuilderMaterials materials, final Map<String, TableMetaData> tables) throws SQLException {

Review comment:
       I just moved this method to the bottom, since it was in the methods belong to 'buildActualTableMetaDataMap'.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread

Review comment:
       OK

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       I will extract these code to method 'appendRemainRuleLogicTables', and ruleLogicTables is useful for exclude loaded tables.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);
+
+        // Append more tables in 'TableContainedRule' by multi-thread
+        Collection<String> toAppendLoadTables = ruleLogicTables.stream()
+                .filter(table -> !result.containsKey(table))
+                .collect(Collectors.toList());
+        if (CollectionUtils.isNotEmpty(toAppendLoadTables)) {
+            Collection<Future<Optional<TableMetaData>>> futures = new ArrayList<>(toAppendLoadTables.size());

Review comment:
       OK.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tristaZero commented on pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

tristaZero commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-893238120


   Hi @Beyondeclipse Thanks for this PR, I know it will be a big task but definitely an important enhancement. If any issues, welcome your discussion with @strongduanmu . We will provide the assistance we can.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] tristaZero commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

tristaZero commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-886031884


   @Beyondeclipse Thanks for your repeated improvement, Look forward to merging it.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-883264104


   @Beyondeclipse Please add test case for your 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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on a change in pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r673981572



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -84,43 +89,45 @@
     
     private static Map<String, TableMetaData> buildActualTableMetaDataMap(final SchemaBuilderMaterials materials) throws SQLException {
         Map<String, TableMetaData> result = new HashMap<>(materials.getRules().size(), 1);
-        appendRemainTables(materials, result);
-        for (ShardingSphereRule rule : materials.getRules()) {
-            if (rule instanceof TableContainedRule) {
-                for (String table : ((TableContainedRule) rule).getTables()) {
-                    if (!result.containsKey(table)) {
-                        TableMetaDataBuilder.load(table, materials).map(optional -> result.put(table, optional));
+        Collection<String> ruleLogicTables = materials.getRules().stream()
+                .filter(rule -> rule instanceof TableContainedRule)
+                .flatMap(rule -> ((TableContainedRule) rule).getTables().stream())
+                .collect(Collectors.toSet());
+        appendRemainTables(materials, result, ruleLogicTables);

Review comment:
       > @Beyondeclipse If you adjust the order of execution, the result parameter will contain the rule table.
   
   Can you give more explanation ?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11345: Improve loading TableMetaData

Posted by GitBox <gi...@apache.org>.

strongduanmu commented on a change in pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#discussion_r677924972



##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -76,109 +69,87 @@
         Map<String, TableMetaData> actualTableMetaMap = buildActualTableMetaDataMap(materials);
         Map<String, TableMetaData> logicTableMetaMap = buildLogicTableMetaDataMap(materials, actualTableMetaMap);
         Map<TableMetaData, TableMetaData> tableMetaDataMap = new HashMap<>(actualTableMetaMap.size(), 1);
-        for (Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
+        for (Map.Entry<String, TableMetaData> entry : actualTableMetaMap.entrySet()) {
             tableMetaDataMap.put(entry.getValue(), logicTableMetaMap.getOrDefault(entry.getKey(), entry.getValue()));
         }
         return tableMetaDataMap;
     }
-    

Review comment:
       Same problem.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/SchemaBuilder.java
##########
@@ -64,7 +57,7 @@
     static {
         ShardingSphereServiceLoader.register(DialectTableMetaDataLoader.class);
     }
-    
+

Review comment:
       @Beyondeclipse Please keep indents consistent with the previous one.




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shardingsphere] Beyondeclipse commented on pull request #11345: improve loading TableMetaData by Multi-Thread

Posted by GitBox <gi...@apache.org>.

Beyondeclipse commented on pull request #11345:
URL: https://github.com/apache/shardingsphere/pull/11345#issuecomment-882160178


   > @Beyondeclipse Can you take a look at the ci exception first? There are failed test cases in the integration test.
   
   Yes, I am looking at it.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org