Impala Public Jenkins has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/12660 )
Change subject: IMPALA-8250: Clean up JNI warnings.
......................................................................
IMPALA-8250: Clean up JNI warnings.
Using LIBHDFS_OPTS+="-Xcheck:jni" revealed a handful of warnings related to
(a) checking for exceptions and (b) leaking local references.
Checking for exceptions required sprinkling RETURN_ERROR_IF_EXC
left and right. I chose not to expand the JniCall infrastructure
to handle this more generally at the moment.
The leaky local references are a bit harder. In the logs, they show up
as "WARNING: JNI local refs: 2597, exceeds capacity: 35" or similar. A
few of these errors seem to be not in our code. The ones that I've
found in our code stemmed from HBaseTableScanner::GetRowKey(): this
method uses local references and wasn't returning them. Using a
JniLocalFrame seems to have taken care of the warnings.
I have added code to skip test_large_strings when JNI checking is
enabled. This test takes forever (presumably because JNI is checking
bounds on strings very aggressively), and times out. The time out also
causes some metric-related checks to fail (since a query is still in
flight).
Debugging this required customizing my JDK to give stack traces
when these warnings occurred. The following diff facilitated
this.
diff -r 76a9c9cf14f1 src/share/vm/prims/jniCheck.cpp
--- a/src/share/vm/prims/jniCheck.cpp Tue Jan 15 10:43:31 2019 +0000
+++ b/src/share/vm/prims/jniCheck.cpp Wed Feb 27 11:57:13 2019 -0800
@@ -143,11 +143,30 @@
static const char * fatal_instance_field_mismatch = "Field type (instance)
mismatch in JNI get/set field operations";
static const char * fatal_non_string = "JNI string operation received a
non-string";
+// thisone: whether to print every time, or maybe, depending on future
+// how many future stacks we want printed (totally racy); helps catch
+// missing exception handling if there's a way to tickle that code
+// reliably.
+static inline void dump_native_stack(JavaThread* thr, bool thisone, int
future) {
+ static int fut_stacks = 0; // racy!
+ if (fut_stacks > 0) {
+ thisone = true;
+ fut_stacks--;
+ }
+ if (future > 0) fut_stacks = future;
+ if (thisone) {
+ frame fr = os::current_frame();
+ char buf[6000];
+ tty->print_cr("Thread: %s %d", thr->get_thread_name(),
thr->osthread()->thread_id());
+ print_native_stack(tty, fr, thr, buf, sizeof(buf));
+ }
+}
// When in VM state:
static void ReportJNIWarning(JavaThread* thr, const char *msg) {
tty->print_cr("WARNING in native method: %s", msg);
thr->print_stack();
+ dump_native_stack(thr, true, 0);
}
// When in NATIVE state:
@@ -199,11 +218,14 @@
tty->print_cr("WARNING in native method: JNI call made without
checking exceptions when required to from %s",
thr->get_pending_jni_exception_check());
thr->print_stack();
+ dump_native_stack(thr, true, 10);
)
thr->clear_pending_jni_exception_check(); // Just complain once
}
}
+
+
/**
* Add to the planned number of handles. I.e. plus current live & warning
threshold
*/
@@ -254,9 +276,12 @@
tty->print_cr("WARNING: JNI local refs: %zu, exceeds capacity: %zu",
live_handles, planned_capacity);
thr->print_stack();
+ dump_native_stack(thr, true, 0);
)
// Complain just the once, reset to current + warn threshold
add_planned_handle_capacity(handles, 0);
+ } else {
+ dump_native_stack(thr, false, 0);
}
}
Change-Id: Idd1709f749a764c1d947704bc64306493863b45f
Reviewed-on: http://gerrit.cloudera.org:8080/12660
Reviewed-by: Joe McDonnell <joemcdonn...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
---
M be/src/catalog/catalog.cc
M be/src/exec/hbase-table-scanner.cc
M be/src/exprs/hive-udf-call.cc
M be/src/runtime/hbase-table-factory.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M tests/query_test/test_insert.py
7 files changed, 55 insertions(+), 10 deletions(-)
Approvals:
Joe McDonnell: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12660
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idd1709f749a764c1d947704bc64306493863b45f
Gerrit-Change-Number: 12660
Gerrit-PatchSet: 5
Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
