[Impala-ASF-CR] [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls

["Philip Zeyliger (Code Review)" <ge...@cloudera.org>]

Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10696 )

Change subject: [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10696/3/be/src/kudu/util/debug-util.cc
File be/src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/10696/3/be/src/kudu/util/debug-util.cc@62
PS3, Line 62: DEFINE_int32(stack_trace_sig_num, SIGRTMIN, "Signal used by the thread watchdog to "
> Firstly, these changes in Kudu's util code are expected to be pushed to the
I'm all for configurability in Kudu's code. My point is that Impala users have no possible reason to configure this. (It's useful to turn it off entirely perhaps, but choosing which signal to use makes no sense to an end user.)


http://gerrit.cloudera.org:8080/#/c/10696/3/be/src/kudu/util/debug-util.cc@288
PS3, Line 288:       ret << g_comm.jstack.Stringify();
> We do have a kill switch for this whole watchdog feature. Do we need to hav
I'm fine with just having one big one.


http://gerrit.cloudera.org:8080/#/c/10696/3/be/src/service/CMakeLists.txt
File be/src/service/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/10696/3/be/src/service/CMakeLists.txt@53
PS3, Line 53: # Jvmti agent library that registers with the JVM on load.
> I don't think there is much overhead in the binary size. I'm not sure I und
225K is true is fine. That's in a statically linked build?

See https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#JNJI_OnLoad and look for "JNI_OnLoad_L".


http://gerrit.cloudera.org:8080/#/c/10696/3/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/10696/3/bin/impala-config.sh@619
PS3, Line 619: LIBHDFS_OPTS+=" -agentpath:$IMPALA_HOME/be/build/latest/service/libjvmtiagent.so"
> I commented about the binary sizes in the other file. 
If we at all can, we should avoid placing additional burden on our users to configure things that they shouldn't need to configure. If we want our binary to always run with this agent, we should configure it programmatically if at all possible.

I think the reason it may be painful is because of HDFS-12628, but we should look into it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28e918761c120043d332b034450208eaf34e3e2b
Gerrit-Change-Number: 10696
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Balazs Jeszenszky <je...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 21:39:12 +0000
Gerrit-HasComments: Yes