Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18772 )
Change subject: IMPALA-11401,IMPALA-10794: Add logs and thread names for catalogd RPCs ...................................................................... Patch Set 5: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2633 PS5, Line 2633: ThreadNameAnnotator tna = new ThreadNameAnnotator("DROP " + tableName We have already created a ThreadNameAnnotator in JniCatalog.ExecDdl, right? Is it useful to do it twice? http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/service/JniCatalog.java@227 PS5, Line 227: try { Is there a reason for creating a ThreadAnnotator in some cases, but not doing it in others? http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/service/JniCatalog.java@261 PS5, Line 261: LOG.info("finished execDdl request: " + shortDesc); optional: logging the duration could be useful (or we do this somewhere else?) http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java File fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java: http://gerrit.cloudera.org:8080/#/c/18772/5/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java@133 PS5, Line 133: if (req.isSetDb_name()) { : if (req.is_refresh) cmd += "FUNCTIONS IN "; : cmd += "DATABASE " + req.getDb_name(); : } else if (req.isSetTable_name()) { Is this the right order to check these? So if we are refreshing a table, won't it set both db name and table name? -- To view, visit http://gerrit.cloudera.org:8080/18772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac7f2eda8b95643a3d3c3bef64ea71b67b20595a Gerrit-Change-Number: 18772 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Comment-Date: Thu, 04 Aug 2022 13:57:29 +0000 Gerrit-HasComments: Yes
