Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9053 )

Change subject: IMPALA-6416: extend Thread::Create to track instance id
......................................................................


Patch Set 3:

(4 comments)

thanks!

http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info-test.cc
File be/src/common/thread-debug-info-test.cc:

http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info-test.cc@70
PS3, Line 70: S
> nit: lowercase S
woops :) Done.


http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info-test.cc@72
PS3, Line 72:   // Child should set its parent_thread_name_ to the parent's 
thread_name_
> Stale comment?
yes, deleted it and added comments about the new behavior


http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info-test.cc@75
PS3, Line 75: parent
> parent_name for consistency with parent_* below? That would make the EXPECT
Renamed it to parent_name.
Also renamed 'child' below to 'child_name'.


http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info.h
File be/src/common/thread-debug-info.h:

http://gerrit.cloudera.org:8080/#/c/9053/3/be/src/common/thread-debug-info.h@114
PS3, Line 114:     const ThreadDebugInfo* thread_debug_info_ = nullptr;
> Nit: I'm not sure how helpful it would be, but you could order everything i
Could you elaborate more on that?
To my understanding all data member has fix width. However, we could think of 
the fix-sized arrays in a way that they contain variable-length data. But, they 
are (instance_id_ and thread_name_) already at the end of the TDI class.



--
To view, visit http://gerrit.cloudera.org:8080/9053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27de2962cf0b224c17b685d77dcba3bf2e9db187
Gerrit-Change-Number: 9053
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Feb 2018 16:47:53 +0000
Gerrit-HasComments: Yes

Reply via email to