Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23436 )
Change subject: IMPALA-14447: Parallelize table loading in getMissingTables() ...................................................................... Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/23436/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23436/11//COMMIT_MSG@23 PS11, Line 23: Testing: > Any performance resutls to share here? I don't. But Quanlong (who report this issue in the first place) has benchmark setup that should be sped up by this patch. http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@403 PS11, Line 403: } finally { > Should catch RejectedExecutionException and fall through to original logic. Ack. Will address this in next patch set. http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java File fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java: http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java@118 PS11, Line 118: Map<String, FeDb> dbs = new ConcurrentHashMap<>(); > This probably does not need to be ConcurrentHashMap since loading is synchr Loading is synchronized here, but reading is not (see L132 for the atomic assignment). Thus, it need to be ConcurrentHashMap. http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java File fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java: http://gerrit.cloudera.org:8080/#/c/23436/11/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java@202 PS11, Line 202: Map<String, FeTable> newMap = new ConcurrentHashMap<>(); > This probably does not need to be ConcurrentHashMap since loading is synchr Loading is synchronized here, but reading is not (see L213 for the atomic assignment). Thus, it need to be ConcurrentHashMap. -- To view, visit http://gerrit.cloudera.org:8080/23436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97a5165844ae846b28338d62e93a20121488d79f Gerrit-Change-Number: 23436 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 06 Oct 2025 20:40:27 +0000 Gerrit-HasComments: Yes
