Paul Rogers 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 2:

(6 comments)

Addressed review comments and rebased.

http://gerrit.cloudera.org:8080/#/c/11228/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11228/2//COMMIT_MSG@24
PS2, Line 24: This patch is tricky to automate tests for, but I verified it 
manually
            : by jstacking a catalogd while performing some workload. Also 
added a
            : simple unit test to verify the thread renaming behavior
> Can be removed I guess.
Done


http://gerrit.cloudera.org:8080/#/c/11228/2/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/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@301
PS2, Line 301:     long end;
> Annotate here? This is one of those common interesting entry points for tab
Done


http://gerrit.cloudera.org:8080/#/c/11228/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2185
PS2, Line 2185:   private TGetPartialCatalogObjectResponse 
doGetPartialCatalogObject(
> This is another interesting entry point RPC for catalog v2 stuff. Add some
Done


http://gerrit.cloudera.org:8080/#/c/11228/2/fe/src/test/java/org/apache/impala/util/ThreadNameAnnotatorTest.java
File fe/src/test/java/org/apache/impala/util/ThreadNameAnnotatorTest.java:

http://gerrit.cloudera.org:8080/#/c/11228/2/fe/src/test/java/org/apache/impala/util/ThreadNameAnnotatorTest.java@24
PS2, Line 24: public class ThreadNameAnnotatorTest {
> great test :-)
Thanks. Handy little item from the bag-o-tricks...


http://gerrit.cloudera.org:8080/#/c/11228/2/fe/src/test/java/org/apache/impala/util/ThreadNameAnnotatorTest.java@44
PS2, Line 44:         wait();
> nit: I hope nothing hangs here forever due to a faulty test? Add a largish
Done


http://gerrit.cloudera.org:8080/#/c/11228/2/fe/src/test/java/org/apache/impala/util/ThreadNameAnnotatorTest.java@107
PS2, Line 107:   public void testExternalRename() throws InterruptedException {
> nit: Add a doc of what it does? Probably difficult to understand without an
Done



--
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: 2
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: Paul Rogers <prog...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Wed, 20 Feb 2019 02:12:17 +0000
Gerrit-HasComments: Yes

Reply via email to