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 4:


File be/src/common/thread-debug-info.h:

PS4, Line 115: system_thread_id_
> if that matches thread_debug_info_->system_thread_id_, why store it here al
I think it is possible that the parent TDI object gets destroyed by the time 
the minidump is generated, but the parent thread is still running.

Maybe the parent thread is a member of a thread pool, and it didn't wait for 
its child thread to finish and now it is already working on something else. 
From the system thread id at least we'll know which thread pool created the 
child thread.

PS4, Line 119:   boost::thread::id boost_thread_id_;
             :   int64_t system_thread_id_ = 0;
> are these related? are they both needed for debugging or is it possible to
boost::thread::id is less prone to being recycled, but I think this property 
doesn't add much value currently.

If we had stored the boost::thread::id in ParentInfo, then we could have some 
use of it. E.g. if the parent TDI is invalid we could use the system thread id 
to switch to the parent thread quickly (if it's still running), then with some 
luck we can check the boost thread id (maybe with the help of a new TDI), and 
from that we could tell if this thread is really the same thread that created 
the child.

So, we have two options here, delete the use of boost::thread::id, or use it 
more extensively by adding it to ParentInfo as well. Do you have a preference?

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: 4
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@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: Mon, 12 Feb 2018 13:35:21 +0000
Gerrit-HasComments: Yes

Reply via email to