impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mi...@apache.org
Subject [6/6] impala git commit: IMPALA-3703: Store query context in thread-local variables
Date Mon, 18 Dec 2017 17:55:59 GMT
IMPALA-3703: Store query context in thread-local variables

This commit introduces the ThreadDebugInfo class which can
hold information about the current thread that can be useful
in debug sessions. It needs to be allocated on the stack of
each thread in order to include it to minidumps.

Currently a ThreadDebugInfo object is created in
Thread::SuperviseThread. This object is available in all
the child stack frames through the global function
GetThreadDebugInfo().

ThreadDebugInfo has members for the thread name and instance
id. These are fixed size char buffers.

If you have a core dump written by Impala, you can locate the
ThreadDebugInfo for the current thread through the global
pointer impala::thread_debug_info.

In a core file that has been created from a minidump, we need
to select the stack frame that allocated the ThreadDebugInfo
object in order to inspect it. It is currently allocated in
Thread::SuperviseThread().

We can use printf in gdb to print the members, e.g.:
printf "%s\n" thread_debug_info.instance_id

Currently the thread name and instance id is stored.

I created some tests in thread-debug-info-test.cc

Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Reviewed-on: http://gerrit.cloudera.org:8080/8621
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/8047b1dc
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/8047b1dc
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/8047b1dc

Branch: refs/heads/master
Commit: 8047b1dcb421994e67e6db412529159dde2f40b5
Parents: 1f7b3b0
Author: Zoltan Borok-Nagy <boroknagyz@cloudera.com>
Authored: Tue Nov 21 14:40:07 2017 +0100
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Mon Dec 18 15:32:59 2017 +0000

----------------------------------------------------------------------
 be/src/common/CMakeLists.txt              |   2 +
 be/src/common/thread-debug-info-test.cc   |  71 ++++++++++++++++
 be/src/common/thread-debug-info.cc        |  41 ++++++++++
 be/src/common/thread-debug-info.h         | 107 +++++++++++++++++++++++++
 be/src/exec/blocking-join-node.cc         |   2 +
 be/src/exec/hdfs-scan-node.cc             |   7 +-
 be/src/runtime/fragment-instance-state.cc |   6 +-
 be/src/runtime/query-state.cc             |   3 +
 be/src/util/thread.cc                     |   4 +
 9 files changed, 241 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/common/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/be/src/common/CMakeLists.txt b/be/src/common/CMakeLists.txt
index 8ba8ca9..c01d71c 100644
--- a/be/src/common/CMakeLists.txt
+++ b/be/src/common/CMakeLists.txt
@@ -32,6 +32,7 @@ add_library(Common
   logging.cc
   status.cc
   kudu_version.cc
+  thread-debug-info.cc
   ${VERSION_CC_GEN_OUTPUT}
 )
 
@@ -49,6 +50,7 @@ add_library(GlobalFlags
 add_dependencies(GlobalFlags gen-deps)
 
 ADD_BE_TEST(atomic-test)
+ADD_BE_TEST(thread-debug-info-test)
 
 # Generate config.h from config.h.in, filling in variables from CMake
 CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/config.h.in

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/common/thread-debug-info-test.cc
----------------------------------------------------------------------
diff --git a/be/src/common/thread-debug-info-test.cc b/be/src/common/thread-debug-info-test.cc
new file mode 100644
index 0000000..1700c48
--- /dev/null
+++ b/be/src/common/thread-debug-info-test.cc
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include "common/thread-debug-info.h"
+#include "testutil/gtest-util.h"
+
+#include "common/names.h"
+
+namespace impala {
+
+TEST(ThreadDebugInfo, Ids) {
+  // This test checks if SetInstanceId() stores the
+  // string representation of a TUniqueId correctly.
+  ThreadDebugInfo thread_debug_info;
+  TUniqueId uid;
+  uid.hi = 123;
+  uid.lo = 456;
+  thread_debug_info.SetInstanceId(uid);
+  string uid_str = PrintId(uid);
+
+  EXPECT_EQ(uid_str, thread_debug_info.GetInstanceId());
+}
+
+TEST(ThreadDebugInfo, ThreadName) {
+  // Checks if we can store the thread name.
+  // If thread name is too long, the prefix of the thread name is stored.
+  ThreadDebugInfo thread_debug_info;
+  string thread_name = "thread-1";
+  thread_debug_info.SetThreadName(thread_name);
+
+  EXPECT_EQ(thread_name, thread_debug_info.GetThreadName());
+
+  string a_255(255, 'a');
+  string b_255(255, 'b');
+  string a_b = a_255 + b_255;
+  thread_debug_info.SetThreadName(a_b);
+  string stored_name = thread_debug_info.GetThreadName();
+
+  // Let's check if we stored the corrects parts of thread name
+  string expected = a_255.substr(0, 244) + "..." + b_255.substr(0, 8);
+  EXPECT_EQ(expected, stored_name);
+}
+
+TEST(ThreadDebugInfo, Global) {
+  // Checks if the constructor of the local ThreadDebugInfo object set the
+  // global pointer to itself.
+  ThreadDebugInfo thread_debug_info;
+  ThreadDebugInfo* global_thread_debug_info = GetThreadDebugInfo();
+
+  EXPECT_EQ(&thread_debug_info, global_thread_debug_info);
+}
+
+}
+
+IMPALA_TEST_MAIN();

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/common/thread-debug-info.cc
----------------------------------------------------------------------
diff --git a/be/src/common/thread-debug-info.cc b/be/src/common/thread-debug-info.cc
new file mode 100644
index 0000000..5e642e6
--- /dev/null
+++ b/be/src/common/thread-debug-info.cc
@@ -0,0 +1,41 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "thread-debug-info.h"
+
+namespace impala {
+
+thread_local ThreadDebugInfo* thread_debug_info;
+
+void ThreadDebugInfo::InitializeThreadDebugInfo(
+    ThreadDebugInfo* thread_debug_info_param) {
+  DCHECK(thread_debug_info == nullptr);
+  thread_debug_info = thread_debug_info_param;
+}
+
+void ThreadDebugInfo::CloseThreadDebugInfo() {
+  DCHECK(thread_debug_info != nullptr);
+  thread_debug_info = nullptr;
+}
+
+ThreadDebugInfo* GetThreadDebugInfo() {
+  DCHECK(thread_debug_info != nullptr);
+  return thread_debug_info;
+}
+
+}
+

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/common/thread-debug-info.h
----------------------------------------------------------------------
diff --git a/be/src/common/thread-debug-info.h b/be/src/common/thread-debug-info.h
new file mode 100644
index 0000000..6219ee8
--- /dev/null
+++ b/be/src/common/thread-debug-info.h
@@ -0,0 +1,107 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef IMPALA_COMMON_THREAD_DEBUG_INFO_H
+#define IMPALA_COMMON_THREAD_DEBUG_INFO_H
+
+#include <string>
+
+#include "glog/logging.h"
+#include "gutil/macros.h"
+#include "util/debug-util.h"
+
+#include "common/names.h"
+
+namespace impala {
+
+/// Stores information about the current thread that can be useful in a debug session.
+/// An object of this class needs to be allocated on the stack in order to include
+/// it in minidumps. While this object is alive, it is available through the global
+/// function 'GetThreadDebugInfo()'.
+/// During a debug session, locate this object in the core file or in the minidump,
+/// then inspect its members.
+class ThreadDebugInfo {
+public:
+  /// Only one ThreadDebugInfo object can be alive per thread at a time.
+  /// This object is not copyable, nor movable
+  ThreadDebugInfo() {
+    // This call makes the global (thread local) pointer point to this object.
+    InitializeThreadDebugInfo(this);
+  }
+
+  ~ThreadDebugInfo() {
+    // Resets the global (thread local) pointer to null.
+    CloseThreadDebugInfo();
+  }
+
+  const char* GetInstanceId() const { return instance_id_; }
+  const char* GetThreadName() const { return thread_name_; }
+
+  /// Saves the string representation of param 'instance_id' to member 'instance_id_'
+  void SetInstanceId(const TUniqueId& instance_id) {
+    string id_str = PrintId(instance_id);
+    DCHECK_LT(id_str.length(), TUNIQUE_ID_STRING_SIZE);
+    id_str.copy(instance_id_, id_str.length());
+  }
+
+  /// Saves param 'thread_name' to member 'thread_name_'.
+  /// If the length of param 'thread_name' is larger than THREAD_NAME_SIZE,
+  /// we store the front of 'thread_name' + '...' + the last few bytes
+  /// of thread name, e.g.: "Long Threadname with more te...001afec4)"
+  void SetThreadName(const string& thread_name) {
+    const int64_t length = thread_name.length();
+
+    if (length < THREAD_NAME_SIZE) {
+      thread_name.copy(thread_name_, length);
+    } else {
+      const int64_t tail_length = THREAD_NAME_TAIL_LENGTH;
+      // 4 is the length of "..." and '\0'
+      const int64_t front_length = THREAD_NAME_SIZE - tail_length - 4;
+      // copy 'front_length' sized front of 'thread_name' to 'thread_name_'
+      thread_name.copy(thread_name_, front_length);
+      // append "..."
+      for (int i = 0; i < 3; ++i) thread_name_[front_length + i] = '.';
+      // append 'tail_length' sized tail of 'thread_name' to 'thread_name_'
+      thread_name.copy(thread_name_ + front_length + 3, tail_length,
+          length - tail_length);
+    }
+  }
+
+private:
+  /// Initializes a thread local pointer with thread_debug_info.
+  static void InitializeThreadDebugInfo(ThreadDebugInfo* thread_debug_info);
+  /// Resets the thread local pointer to nullptr.
+  static void CloseThreadDebugInfo();
+
+  static constexpr int64_t TUNIQUE_ID_STRING_SIZE = 34;
+  static constexpr int64_t THREAD_NAME_SIZE = 256;
+  static constexpr int64_t THREAD_NAME_TAIL_LENGTH = 8;
+
+  char instance_id_[TUNIQUE_ID_STRING_SIZE] = {};
+  char thread_name_[THREAD_NAME_SIZE] = {};
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadDebugInfo);
+};
+
+/// Returns a pointer to the ThreadDebugInfo object for this thread.
+/// The ThreadDebugInfo object needs to be created before this function is called.
+ThreadDebugInfo* GetThreadDebugInfo();
+
+}
+
+#endif
+

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/exec/blocking-join-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/blocking-join-node.cc b/be/src/exec/blocking-join-node.cc
index 7adea7f..944aba8 100644
--- a/be/src/exec/blocking-join-node.cc
+++ b/be/src/exec/blocking-join-node.cc
@@ -19,6 +19,7 @@
 
 #include <sstream>
 
+#include "common/thread-debug-info.h"
 #include "exec/data-sink.h"
 #include "exprs/scalar-expr.h"
 #include "runtime/fragment-instance-state.h"
@@ -195,6 +196,7 @@ Status BlockingJoinNode::ProcessBuildInputAndOpenProbe(
     unique_ptr<Thread> build_thread;
     Status thread_status = Thread::Create(FragmentInstanceState::FINST_THREAD_GROUP_NAME,
         thread_name, [this, state, build_sink, status=&build_side_status]() {
+          GetThreadDebugInfo()->SetInstanceId(state->fragment_instance_id());
           ProcessBuildInputAsync(state, build_sink, status);
         }, &build_thread, true);
     if (!thread_status.ok()) {

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/exec/hdfs-scan-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scan-node.cc b/be/src/exec/hdfs-scan-node.cc
index 710a8af..235c799 100644
--- a/be/src/exec/hdfs-scan-node.cc
+++ b/be/src/exec/hdfs-scan-node.cc
@@ -21,6 +21,7 @@
 #include <sstream>
 
 #include "common/logging.h"
+#include "common/thread-debug-info.h"
 #include "exec/base-sequence-scanner.h"
 #include "exec/hdfs-scanner.h"
 #include "exec/scanner-context.h"
@@ -347,7 +348,11 @@ void HdfsScanNode::ThreadTokenAvailableCb(ThreadResourceMgr::ResourcePool*
pool)
         PrintId(runtime_state_->fragment_instance_id()), id(),
         num_scanner_threads_started_counter_->value());
 
-    auto fn = [this]() { this->ScannerThread(); };
+    auto fn = [this]() {
+      RuntimeState* state = this->runtime_state();
+      GetThreadDebugInfo()->SetInstanceId(state->fragment_instance_id());
+      this->ScannerThread();
+    };
     std::unique_ptr<Thread> t;
     status =
       Thread::Create(FragmentInstanceState::FINST_THREAD_GROUP_NAME, name, fn, &t, true);

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/runtime/fragment-instance-state.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/fragment-instance-state.cc b/be/src/runtime/fragment-instance-state.cc
index bf437ba..5c9deb8 100644
--- a/be/src/runtime/fragment-instance-state.cc
+++ b/be/src/runtime/fragment-instance-state.cc
@@ -26,6 +26,7 @@
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 
 #include "common/names.h"
+#include "common/thread-debug-info.h"
 #include "codegen/llvm-codegen.h"
 #include "exec/plan-root-sink.h"
 #include "exec/exec-node.h"
@@ -227,7 +228,10 @@ Status FragmentInstanceState::Prepare() {
     string thread_name = Substitute("profile-report (finst:$0)", PrintId(instance_id()));
     unique_lock<mutex> l(report_thread_lock_);
     RETURN_IF_ERROR(Thread::Create(FragmentInstanceState::FINST_THREAD_GROUP_NAME,
-        thread_name, [this]() { this->ReportProfileThread(); }, &report_thread_, true));
+        thread_name, [this]() {
+          GetThreadDebugInfo()->SetInstanceId(this->instance_id());
+          this->ReportProfileThread();
+        }, &report_thread_, true));
     // Make sure the thread started up, otherwise ReportProfileThread() might get into
     // a race with StopReportThread().
     while (!report_thread_active_) report_thread_started_cv_.Wait(l);

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/runtime/query-state.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/query-state.cc b/be/src/runtime/query-state.cc
index ae207a2..10c8033 100644
--- a/be/src/runtime/query-state.cc
+++ b/be/src/runtime/query-state.cc
@@ -20,6 +20,7 @@
 #include <boost/thread/lock_guard.hpp>
 #include <boost/thread/locks.hpp>
 
+#include "common/thread-debug-info.h"
 #include "exprs/expr.h"
 #include "runtime/backend-client.h"
 #include "runtime/bufferpool/buffer-pool.h"
@@ -372,6 +373,8 @@ void QueryState::ReleaseExecResourceRefcount() {
 }
 
 void QueryState::ExecFInstance(FragmentInstanceState* fis) {
+  GetThreadDebugInfo()->SetInstanceId(fis->instance_id());
+
   ImpaladMetrics::IMPALA_SERVER_NUM_FRAGMENTS_IN_FLIGHT->Increment(1L);
   ImpaladMetrics::IMPALA_SERVER_NUM_FRAGMENTS->Increment(1L);
   VLOG_QUERY << "Executing instance. instance_id=" << PrintId(fis->instance_id())

http://git-wip-us.apache.org/repos/asf/impala/blob/8047b1dc/be/src/util/thread.cc
----------------------------------------------------------------------
diff --git a/be/src/util/thread.cc b/be/src/util/thread.cc
index cd38c9a..536119b 100644
--- a/be/src/util/thread.cc
+++ b/be/src/util/thread.cc
@@ -23,6 +23,7 @@
 #include <sys/syscall.h>
 #include <sys/types.h>
 
+#include "common/thread-debug-info.h"
 #include "util/coding-util.h"
 #include "util/debug-util.h"
 #include "util/error-util.h"
@@ -345,6 +346,9 @@ void Thread::SuperviseThread(const string& name, const string&
category,
   thread_mgr_ref->AddThread(this_thread::get_id(), name_copy, category_copy, system_tid);
   thread_started->Set(system_tid);
 
+  ThreadDebugInfo thread_debug_info;
+  thread_debug_info.SetThreadName(name_copy);
+
   // Any reference to any parameter not copied in by value may no longer be valid after
   // this point, since the caller that is waiting on *tid != 0 may wake, take the lock and
   // destroy the enclosing Thread object.


Mime
View raw message