Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11388 )
Change subject: WIP: IMPALA-7527: add fetch-from-catalogd cache info to profile ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11388/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11388/3//COMMIT_MSG@32 PS3, Line 32: breaking down by category? still hoping to get some feedback on the above http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@189 PS3, Line 189: private static final String DB_LIST_STATS_CATEGORY = "DatabaseList"; > Is it worth having some sort of enum for these keys? Maybe it will make the yea I think it'll add quite some boiler plate without any real gains in safety since the profiles store this stuff in a string map anyway http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/service/FrontendProfile.java File fe/src/main/java/org/apache/impala/service/FrontendProfile.java: http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/service/FrontendProfile.java@151 PS3, Line 151: oldThreadLocalValue_ = THREAD_LOCAL.get(); > Can you explain what is going on here? Is there a case where oldThreadLocal Just felt like it's good practice to treat scopes as nestable and "restore" the prior state after the scope is popped out of. Currently we have a pretty strict 1:1 query:thread kind of mapping but maybe in the future that might not be the case. I can add a Preconditions.checkState that it starts as null prior to entering the scope if you think that's cleaner until we have some fancier use cases? -- To view, visit http://gerrit.cloudera.org:8080/11388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276 Gerrit-Change-Number: 11388 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 07 Sep 2018 02:22:03 +0000 Gerrit-HasComments: Yes
