Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11228 )
Change subject: IMPALA-7450. Set thread name during refresh/load operations ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/11228/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11228/1//COMMIT_MSG@25 PS1, Line 25: This patch is tricky to automate tests for, but I verified it manually > What do you think of the following test: k, I'll add a simple test. I think just doing it single-threaded and asserting on the expected thread name from the same thread is probably easiest instead of futzing with semaphores. http://gerrit.cloudera.org:8080/#/c/11228/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/11228/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1005 PS1, Line 1005: String annotation = String.format("invalidating metadata - %s/%s dbs complete", > In an ideal world, the RPC from the impalad would be carrying a thread id, yea, distributed tracing is also on my list, but wanted to start with this simple incremental improvement before I open the can of worms of which framework to use (eg opentracing vs opencensus) http://gerrit.cloudera.org:8080/#/c/11228/1/fe/src/main/java/org/apache/impala/util/ThreadNameAnnotator.java File fe/src/main/java/org/apache/impala/util/ThreadNameAnnotator.java: http://gerrit.cloudera.org:8080/#/c/11228/1/fe/src/main/java/org/apache/impala/util/ThreadNameAnnotator.java@48 PS1, Line 48: public void close() { > Do we want this to also double as something that logs long requests? i.e., maybe I'll evolve this class into something a bit more broad like: try (InstrumentedScope s = new InstrumentedScope("blah blah") .annotateThreadName() .logIfSlow(1, TimeUnit.SECONDS) .incrementMetric(METRIC_LOAD_TIME) .addToProfile("FooTime")) etc. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/11228 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7c850d6bb2eedc375ee567c19eb17add335f60c Gerrit-Change-Number: 11228 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 15 Aug 2018 23:46:50 +0000 Gerrit-HasComments: Yes
