IMPALA-6436: exit instead of abort for catalog startup failure Rename EXIT_WITH_EXC to ABORT_WITH_EXC to make the behaviour more obvious at callsites.
Handle exceptions from Catalog constructor by logging the backtrace and exiting cleanly, rather than aborting. This will prevent generation of a coredump or minidump. Testing: Tested starting the catalogd locally without the HMS running and a low connection timeout: start-impala-cluster.py --catalogd_args=--initial_hms_cnxn_timeout_s=2 Confirmed that the backtrace was logged to catalogd.ERROR and that no core or minidump was generated. Change-Id: I4026dccb39843b847426112fc0fe9ba897e48dcc Reviewed-on: http://gerrit.cloudera.org:8080/11871 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/0d4c6ae0 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0d4c6ae0 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0d4c6ae0 Branch: refs/heads/branch-3.1.0 Commit: 0d4c6ae0519873dfff3b9859dd17ba25b7419b07 Parents: f90931e Author: Tim Armstrong <[email protected]> Authored: Fri Nov 2 09:46:29 2018 -0700 Committer: Zoltan Borok-Nagy <[email protected]> Committed: Tue Nov 13 12:50:23 2018 +0100 ---------------------------------------------------------------------- be/src/catalog/catalog.cc | 4 +-- be/src/scheduling/request-pool-service.cc | 10 ++++---- be/src/service/fe-support.cc | 2 +- be/src/service/frontend.cc | 4 +-- be/src/util/jni-util.h | 35 ++++++++++++++++++++------ be/src/util/logging-support.cc | 4 +-- be/src/util/zip-util.cc | 2 +- 7 files changed, 40 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/catalog/catalog.cc ---------------------------------------------------------------------- diff --git a/be/src/catalog/catalog.cc b/be/src/catalog/catalog.cc index a2d5f4b..e5bf35c 100644 --- a/be/src/catalog/catalog.cc +++ b/be/src/catalog/catalog.cc @@ -76,7 +76,7 @@ Catalog::Catalog() { JNIEnv* jni_env = getJNIEnv(); // Create an instance of the java class JniCatalog catalog_class_ = jni_env->FindClass("org/apache/impala/service/JniCatalog"); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); uint32_t num_methods = sizeof(methods) / sizeof(methods[0]); for (int i = 0; i < num_methods; ++i) { @@ -87,7 +87,7 @@ Catalog::Catalog() { ABORT_IF_ERROR(GetThriftBackendGflags(jni_env, &cfg_bytes)); jobject catalog = jni_env->NewObject(catalog_class_, catalog_ctor_, cfg_bytes); - EXIT_IF_EXC(jni_env); + CLEAN_EXIT_IF_EXC(jni_env); ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, catalog, &catalog_)); } http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/scheduling/request-pool-service.cc ---------------------------------------------------------------------- diff --git a/be/src/scheduling/request-pool-service.cc b/be/src/scheduling/request-pool-service.cc index f9fdf88..c3c9750 100644 --- a/be/src/scheduling/request-pool-service.cc +++ b/be/src/scheduling/request-pool-service.cc @@ -124,7 +124,7 @@ RequestPoolService::RequestPoolService(MetricGroup* metrics) : JNIEnv* jni_env = getJNIEnv(); request_pool_service_class_ = jni_env->FindClass("org/apache/impala/util/RequestPoolService"); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); uint32_t num_methods = sizeof(methods) / sizeof(methods[0]); for (int i = 0; i < num_methods; ++i) { ABORT_IF_ERROR(JniUtil::LoadJniMethod(jni_env, request_pool_service_class_, @@ -133,18 +133,18 @@ RequestPoolService::RequestPoolService(MetricGroup* metrics) : jstring fair_scheduler_config_path = jni_env->NewStringUTF(FLAGS_fair_scheduler_allocation_path.c_str()); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); jstring llama_site_path = jni_env->NewStringUTF(FLAGS_llama_site_path.c_str()); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); jobject request_pool_service = jni_env->NewObject(request_pool_service_class_, ctor_, fair_scheduler_config_path, llama_site_path); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, request_pool_service, &request_pool_service_)); jni_env->CallObjectMethod(request_pool_service_, start_id); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); } Status RequestPoolService::ResolveRequestPool(const TQueryCtx& ctx, http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/service/fe-support.cc ---------------------------------------------------------------------- diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc index 5dd06de..4a25250 100644 --- a/be/src/service/fe-support.cc +++ b/be/src/service/fe-support.cc @@ -699,7 +699,7 @@ void InitFeSupport(bool disable_codegen) { jclass native_backend_cl = env->FindClass("org/apache/impala/service/FeSupport"); env->RegisterNatives(native_backend_cl, native_methods, sizeof(native_methods) / sizeof(native_methods[0])); - EXIT_IF_EXC(env); + ABORT_IF_EXC(env); } } http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/service/frontend.cc ---------------------------------------------------------------------- diff --git a/be/src/service/frontend.cc b/be/src/service/frontend.cc index af45d96..c4ad4c7 100644 --- a/be/src/service/frontend.cc +++ b/be/src/service/frontend.cc @@ -106,7 +106,7 @@ Frontend::Frontend() { JNIEnv* jni_env = getJNIEnv(); // create instance of java class JniFrontend fe_class_ = jni_env->FindClass("org/apache/impala/service/JniFrontend"); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); uint32_t num_methods = sizeof(methods) / sizeof(methods[0]); for (int i = 0; i < num_methods; ++i) { @@ -117,7 +117,7 @@ Frontend::Frontend() { ABORT_IF_ERROR(GetThriftBackendGflags(jni_env, &cfg_bytes)); jobject fe = jni_env->NewObject(fe_class_, fe_ctor_, cfg_bytes); - EXIT_IF_EXC(jni_env); + ABORT_IF_EXC(jni_env); ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, fe, &fe_)); } http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/util/jni-util.h ---------------------------------------------------------------------- diff --git a/be/src/util/jni-util.h b/be/src/util/jni-util.h index 576f943..4b18f20 100644 --- a/be/src/util/jni-util.h +++ b/be/src/util/jni-util.h @@ -59,8 +59,8 @@ #define THROW_IF_EXC(env, exc_class) \ do { \ jthrowable exc = (env)->ExceptionOccurred(); \ - if (exc != NULL) { \ - DCHECK((throwable_to_string_id_) != NULL); \ + if (exc != nullptr) { \ + DCHECK((throwable_to_string_id_) != nullptr); \ jstring stack = (jstring) env->CallStaticObjectMethod(JniUtil::jni_util_class(), \ (JniUtil::throwable_to_stack_trace_id()), exc); \ jboolean is_copy; \ @@ -75,7 +75,7 @@ #define RETURN_IF_EXC(env) \ do { \ jthrowable exc = (env)->ExceptionOccurred(); \ - if (exc != NULL) { \ + if (exc != nullptr) { \ jstring stack = (jstring) env->CallStaticObjectMethod(JniUtil::jni_util_class(), \ (JniUtil::throwable_to_stack_trace_id()), exc); \ jboolean is_copy; \ @@ -86,10 +86,14 @@ } \ } while (false) -#define EXIT_IF_EXC(env) \ +// If there's an exception in 'env', log the backtrace at FATAL level and abort the +// process. This will generate a core dump if core dumps are enabled, so this should +// generally only be called for internal errors where the coredump is useful for +// diagnostics. +#define ABORT_IF_EXC(env) \ do { \ jthrowable exc = (env)->ExceptionOccurred(); \ - if (exc != NULL) { \ + if (exc != nullptr) { \ jstring stack = (jstring) env->CallStaticObjectMethod(JniUtil::jni_util_class(), \ (JniUtil::throwable_to_stack_trace_id()), exc); \ jboolean is_copy; \ @@ -99,10 +103,25 @@ } \ } while (false) +// If there's an exception in 'env', log the backtrace at ERROR level and exit the process +// cleanly with status 1. +#define CLEAN_EXIT_IF_EXC(env) \ + do { \ + jthrowable exc = (env)->ExceptionOccurred(); \ + if (exc != nullptr) { \ + jstring stack = (jstring) env->CallStaticObjectMethod(JniUtil::jni_util_class(), \ + (JniUtil::throwable_to_stack_trace_id()), exc); \ + jboolean is_copy; \ + const char* c_stack = \ + reinterpret_cast<const char*>((env)->GetStringUTFChars(stack, &is_copy)); \ + CLEAN_EXIT_WITH_ERROR(c_stack); \ + } \ + } while (false) + #define RETURN_ERROR_IF_EXC(env) \ do { \ jthrowable exc = (env)->ExceptionOccurred(); \ - if (exc != NULL) return JniUtil::GetJniExceptionMsg(env);\ + if (exc != nullptr) return JniUtil::GetJniExceptionMsg(env);\ } while (false) /// C linkage for helper functions in hdfsJniHelper.h @@ -118,8 +137,8 @@ class Status; /// the corresponding objects. class JniLocalFrame { public: - JniLocalFrame(): env_(NULL) {} - ~JniLocalFrame() { if (env_ != NULL) env_->PopLocalFrame(NULL); } + JniLocalFrame(): env_(nullptr) {} + ~JniLocalFrame() { if (env_ != nullptr) env_->PopLocalFrame(nullptr); } JniLocalFrame(JniLocalFrame&& other) noexcept : env_(other.env_) { http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/util/logging-support.cc ---------------------------------------------------------------------- diff --git a/be/src/util/logging-support.cc b/be/src/util/logging-support.cc index 457c8f9..b050f37 100644 --- a/be/src/util/logging-support.cc +++ b/be/src/util/logging-support.cc @@ -110,7 +110,7 @@ Webserver::UrlCallback MakeCallback(const F& fnc, bool display_log4j_handlers) { void InitDynamicLoggingSupport() { JNIEnv* env = getJNIEnv(); log4j_logger_class_ = env->FindClass("org/apache/impala/util/GlogAppender"); - EXIT_IF_EXC(env); + ABORT_IF_EXC(env); JniMethodDescriptor get_log_level_method_desc = {"getLogLevel", "([B)Ljava/lang/String;", &get_log_level_method}; JniMethodDescriptor set_log_level_method_desc = @@ -240,7 +240,7 @@ void InitJvmLoggingSupport() { nm.signature = const_cast<char*>("(ILjava/lang/String;Ljava/lang/String;I)V"); nm.fnPtr = reinterpret_cast<void*>(::Java_org_apache_impala_util_NativeLogger_Log); env->RegisterNatives(native_backend_cl, &nm, 1); - EXIT_IF_EXC(env); + ABORT_IF_EXC(env); InitDynamicLoggingSupport(); } http://git-wip-us.apache.org/repos/asf/impala/blob/0d4c6ae0/be/src/util/zip-util.cc ---------------------------------------------------------------------- diff --git a/be/src/util/zip-util.cc b/be/src/util/zip-util.cc index f6e1f15..43c46df 100644 --- a/be/src/util/zip-util.cc +++ b/be/src/util/zip-util.cc @@ -30,7 +30,7 @@ jmethodID ZipUtil::extract_files_method; // ZipUtil.extractFiles() void ZipUtil::InitJvm() { JNIEnv* env = getJNIEnv(); zip_util_class_ = env->FindClass("org/apache/impala/util/ZipUtil"); - EXIT_IF_EXC(env); + ABORT_IF_EXC(env); JniMethodDescriptor extract_files_method_desc = {"extractFiles", "([B)V", &extract_files_method};
