Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14846 )
Change subject: IMPALA-9195: Using multithreaded execution to accelerate 'show tables/databases' ...................................................................... Patch Set 7: (3 comments) The implementation looks good to me, but I am concerned about possible performance impact, see my comment in Frontend.java http://gerrit.cloudera.org:8080/#/c/14846/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14846/7//COMMIT_MSG@15 PS7, Line 15: num_check_access_threads I would prefer a name the contains "authorization" or "auth". The current name could mean many things. http://gerrit.cloudera.org:8080/#/c/14846/7//COMMIT_MSG@18 PS7, Line 18: range : of 1 to 32. The default value of 'num_check_access_threads' is 1. nit: wrap at 72 http://gerrit.cloudera.org:8080/#/c/14846/7/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14846/7/fe/src/main/java/org/apache/impala/service/Frontend.java@859 PS7, Line 859: submit I am not familiar with ExecutorService, so I may be wrong here. What happens if SHOW DATABASES is called by several users concurrently? Does MAX_CHECK_ACCESS_POOL_SIZE maximize the number of threads that can work in parallel, or the current thread (where doGetTableNames is called) can also do some work? If no more than MAX_CHECK_ACCESS_POOL_SIZE threads can work at the same time and the default value 1 is used, then this change serializes parallel SHOW DATABASES calls which is a regression compared to the current state. I can imagine that in certain cases bumping MAX_CHECK_ACCESS_POOL_SIZE can help a lot, but we should ensure that the default 1 does not cause significant regression. related stackoverflow question: https://stackoverflow.com/questions/6581188/is-there-an-executorservice-that-uses-the-current-thread -- To view, visit http://gerrit.cloudera.org:8080/14846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I860e0d18afa0421665f8b3b1c5561d6bdacc5e96 Gerrit-Change-Number: 14846 Gerrit-PatchSet: 7 Gerrit-Owner: Zhou Xu <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Zhou Xu <[email protected]> Gerrit-Comment-Date: Mon, 16 Dec 2019 13:14:48 +0000 Gerrit-HasComments: Yes
