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 <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 09 Feb 2018 16:47:53 +0000 Gerrit-HasComments: Yes
