You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/02/21 00:10:42 UTC

[1/3] kudu git commit: Support decimal columns in load generation tool

Repository: kudu
Updated Branches:
  refs/heads/master 9c70e0751 -> 270dd9996


Support decimal columns in load generation tool

Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Reviewed-on: http://gerrit.cloudera.org:8080/9362
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: e0e440eff66b3da7f456ae01b15eefae3fccfd45
Parents: 9c70e07
Author: Grant Henke <gr...@gmail.com>
Authored: Mon Feb 19 17:06:44 2018 -0600
Committer: Grant Henke <gr...@gmail.com>
Committed: Tue Feb 20 20:29:04 2018 +0000

----------------------------------------------------------------------
 src/kudu/tools/kudu-tool-test.cc   |  9 +++++++++
 src/kudu/tools/tool_action_perf.cc | 14 ++++++++++++++
 2 files changed, 23 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/e0e440ef/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index c8cb27f..de8acf8 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -1339,6 +1339,15 @@ void ToolTest::RunLoadgen(int num_tservers,
         ColumnSchema("int64_val", INT64),
         ColumnSchema("float_val", FLOAT),
         ColumnSchema("double_val", DOUBLE),
+        ColumnSchema("decimal32_val", DECIMAL32, false,
+                     NULL, NULL, ColumnStorageAttributes(),
+                     ColumnTypeAttributes(9, 9)),
+        ColumnSchema("decimal64_val", DECIMAL64, false,
+                     NULL, NULL, ColumnStorageAttributes(),
+                     ColumnTypeAttributes(18, 2)),
+        ColumnSchema("decimal128_val", DECIMAL128, false,
+                     NULL, NULL, ColumnStorageAttributes(),
+                     ColumnTypeAttributes(38, 0)),
         ColumnSchema("unixtime_micros_val", UNIXTIME_MICROS),
         ColumnSchema("string_val", STRING),
         ColumnSchema("binary_val", BINARY),

http://git-wip-us.apache.org/repos/asf/kudu/blob/e0e440ef/src/kudu/tools/tool_action_perf.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_perf.cc b/src/kudu/tools/tool_action_perf.cc
index 1e4e30c..7c23bc8 100644
--- a/src/kudu/tools/tool_action_perf.cc
+++ b/src/kudu/tools/tool_action_perf.cc
@@ -137,6 +137,8 @@
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tools/tool_action_common.h"
+#include "kudu/util/decimal_util.h"
+#include "kudu/util/int128.h"
 #include "kudu/util/oid_generator.h"
 #include "kudu/util/random.h"
 #include "kudu/util/status.h"
@@ -350,6 +352,18 @@ Status GenerateRowData(Generator* gen, KuduPartialRow* row,
       case DOUBLE:
         RETURN_NOT_OK(row->SetDouble(idx, gen->Next<double>()));
         break;
+      case DECIMAL32:
+        RETURN_NOT_OK(row->SetUnscaledDecimal(idx, std::min(gen->Next<int32_t>(),
+                                                              kMaxUnscaledDecimal32)));
+        break;
+      case DECIMAL64:
+        RETURN_NOT_OK(row->SetUnscaledDecimal(idx, std::min(gen->Next<int64_t>(),
+                                                            kMaxUnscaledDecimal64)));
+        break;
+      case DECIMAL128:
+        RETURN_NOT_OK(row->SetUnscaledDecimal(idx, std::min(gen->Next<int128_t>(),
+                                                            kMaxUnscaledDecimal128)));
+        break;
       case BINARY:
         if (fixed_string.empty()) {
           RETURN_NOT_OK(row->SetBinary(idx, gen->Next<string>()));


[2/3] kudu git commit: KUDU-2275. Upgrade libunwind to 1.3-rc1

Posted by mp...@apache.org.
KUDU-2275. Upgrade libunwind to 1.3-rc1

Per KUDU-2275, the version of libunwind that we were previously using
can occasionally crash during stack unwinding if it attempts to access a
page which is mprotected to be non-readable. This could be due to
incorrect interpretation of dwarf unwinding info or some other bug.

As we are trying to collect stacks more aggressively now, it's important
to upgrade. This new version uses a more robust mechanism for checking
whether a page is valid to access before accessing it during unwinding.

This also includes a patch from the libunwind git repo which fixes a
potential issue with libunwind in ASAN builds. I didn't run into this
issue yet myself, but seems like a straightforward and necessary fix.

Change-Id: I3f942a0d566e1b1e5ebe48f09362bf8aa3e1e33e
Reviewed-on: http://gerrit.cloudera.org:8080/9335
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: c182d626c111dde30d5602aaba102fb47b7404df
Parents: e0e440e
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Feb 15 09:28:01 2018 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Feb 20 21:34:47 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/file_cache-test.cc                | 13 ++++--
 thirdparty/download-thirdparty.sh               |  7 ++++
 ...rectly-in-write_validate-to-avoid-ASAN.patch | 43 ++++++++++++++++++++
 thirdparty/vars.sh                              |  2 +-
 4 files changed, 61 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/src/kudu/util/file_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache-test.cc b/src/kudu/util/file_cache-test.cc
index 0595e5b..217a2be 100644
--- a/src/kudu/util/file_cache-test.cc
+++ b/src/kudu/util/file_cache-test.cc
@@ -29,8 +29,10 @@
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/cache.h"
+#include "kudu/util/debug-util.h"
 #include "kudu/util/env.h"
 #include "kudu/util/metrics.h"  // IWYU pragma: keep
 #include "kudu/util/random.h"
@@ -55,13 +57,18 @@ template <class FileType>
 class FileCacheTest : public KuduTest {
  public:
   FileCacheTest()
-      : rand_(SeedRandom()),
-        initial_open_fds_(CountOpenFds(env_)) {
+      : rand_(SeedRandom()) {
     // Simplify testing of the actual cache capacity.
     FLAGS_cache_force_single_shard = true;
 
     // Speed up tests that check the number of descriptors.
     FLAGS_file_cache_expiry_period_ms = 1;
+
+    // libunwind internally uses two file descriptors as a pipe.
+    // Make sure it gets initialized early so that our fd count
+    // doesn't get affected by it.
+    ignore_result(GetStackTraceHex());
+    initial_open_fds_ = CountOpenFds(env_);
   }
 
   void SetUp() override {
@@ -96,7 +103,7 @@ class FileCacheTest : public KuduTest {
   }
 
   Random rand_;
-  const int initial_open_fds_;
+  int initial_open_fds_;
   unique_ptr<FileCache<FileType>> cache_;
 };
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/download-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index 3bff35b..ba76d12 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -244,8 +244,15 @@ if [ ! -d $CRCUTIL_SOURCE ]; then
   echo
 fi
 
+LIBUNWIND_PATCHLEVEL=1
+delete_if_wrong_patchlevel $LIBUNWIND_SOURCE $LIBUNWIND_PATCHLEVEL
 if [ ! -d $LIBUNWIND_SOURCE ]; then
   fetch_and_expand libunwind-${LIBUNWIND_VERSION}.tar.gz
+  pushd $LIBUNWIND_SOURCE
+  patch -p1 < $TP_DIR/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
+  touch patchlevel-$LIBUNWIND_PATCHLEVEL
+  popd
+  echo
 fi
 
 if [ ! -d $PYTHON_SOURCE ]; then

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
----------------------------------------------------------------------
diff --git a/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch b/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
new file mode 100644
index 0000000..ef68266
--- /dev/null
+++ b/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
@@ -0,0 +1,43 @@
+From 7d6cc6696ab8a808da3dbe23ca2493ddf2799b56 Mon Sep 17 00:00:00 2001
+From: Dave Watson <da...@fb.com>
+Date: Wed, 17 Jan 2018 08:04:05 -0800
+Subject: [PATCH 1/1] Use syscall directly in write_validate to avoid ASAN
+ errors
+
+ASAN will complain about this write call with the following error:
+
+ERROR: AddressSanitizer: stack-buffer-underflow on address
+HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
+
+This is similar to what google's abseil does to work around the issue.
+
+Reported-by: qiwang@fb.com
+---
+ src/x86_64/Ginit.c | 4 +++-
+ 2 files changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/src/x86_64/Ginit.c b/src/x86_64/Ginit.c
+index 6281b76..2a84a1e 100644
+--- a/src/x86_64/Ginit.c
++++ b/src/x86_64/Ginit.c
+@@ -34,6 +34,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.  */
+ #include <stdlib.h>
+ #include <string.h>
+ #include <sys/mman.h>
++#include <sys/syscall.h>
+ 
+ #include "unwind_i.h"
+ 
+@@ -107,7 +108,8 @@ write_validate (void *addr)
+ 
+   do
+     {
+-      ret = write (mem_validate_pipe[1], addr, 1);
++      /* use syscall insteadof write() so that ASAN does not complain */
++      ret = syscall (SYS_write, mem_validate_pipe[1], addr, 1);
+     }
+   while ( errno == EINTR );
+ 
+-- 
+2.7.4
+

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/vars.sh
----------------------------------------------------------------------
diff --git a/thirdparty/vars.sh b/thirdparty/vars.sh
index 472970c..6937d06 100644
--- a/thirdparty/vars.sh
+++ b/thirdparty/vars.sh
@@ -134,7 +134,7 @@ CRCUTIL_VERSION=42148a6df6986a257ab21c80f8eca2e54544ac4d
 CRCUTIL_NAME=crcutil-$CRCUTIL_VERSION
 CRCUTIL_SOURCE=$TP_SOURCE_DIR/$CRCUTIL_NAME
 
-LIBUNWIND_VERSION=1.1a
+LIBUNWIND_VERSION=1.3-rc1
 LIBUNWIND_NAME=libunwind-$LIBUNWIND_VERSION
 LIBUNWIND_SOURCE=$TP_SOURCE_DIR/$LIBUNWIND_NAME
 


[3/3] kudu git commit: KUDU-2291 (part 2): Add a /stacks page

Posted by mp...@apache.org.
KUDU-2291 (part 2): Add a /stacks page

This adds a simple /stacks web page which dumps the currently running
threads in a plain text format.

Since we often have a lot of idle threadpool threads sitting around in
the "wait for work" state, the output collapses all threads with
matching stacks and only displays the stack once, making it more
suitable for human consumption.

Example output from a local kudu-master:
  https://gist.github.com/b64739ee5fb146ea1953380f57b996c4

Longer term we may want to integrate stack-tracing capability into the
/threadz view as well, but for now I left this as a low-level utility
which doesn't access the thread manager, etc. I left a few TODOs for
further enhancements, but I've already found this helpful for
understanding some perf anomalies while playing with YCSB, so let's get
it committed and improve as we go.

Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Reviewed-on: http://gerrit.cloudera.org:8080/9253
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Mike Percy <mp...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/270dd999
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/270dd999
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/270dd999

Branch: refs/heads/master
Commit: 270dd999683e930d89cfa7865a1f85239b525183
Parents: c182d62
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Feb 7 19:30:39 2018 -0800
Committer: Mike Percy <mp...@apache.org>
Committed: Wed Feb 21 00:07:25 2018 +0000

----------------------------------------------------------------------
 .../integration-tests/linked_list-test-util.h   | 14 +--
 src/kudu/server/default_path_handlers.cc        | 93 ++++++++++++++++++++
 src/kudu/util/debug-util.cc                     |  6 +-
 3 files changed, 105 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/270dd999/src/kudu/integration-tests/linked_list-test-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/linked_list-test-util.h b/src/kudu/integration-tests/linked_list-test-util.h
index 71c6758..3f873ed 100644
--- a/src/kudu/integration-tests/linked_list-test-util.h
+++ b/src/kudu/integration-tests/linked_list-test-util.h
@@ -312,22 +312,24 @@ class PeriodicWebUIChecker {
     // List of master and ts web pages to fetch
     std::vector<std::string> master_pages, ts_pages;
 
-    master_pages.emplace_back("/metrics");
+    master_pages.emplace_back("/dump-entities");
     master_pages.emplace_back("/masters");
+    master_pages.emplace_back("/mem-trackers");
+    master_pages.emplace_back("/metrics");
+    master_pages.emplace_back("/stacks");
     master_pages.emplace_back("/tables");
-    master_pages.emplace_back("/dump-entities");
     master_pages.emplace_back("/tablet-servers");
-    master_pages.emplace_back("/mem-trackers");
 
+    ts_pages.emplace_back("/maintenance-manager");
+    ts_pages.emplace_back("/mem-trackers");
     ts_pages.emplace_back("/metrics");
+    ts_pages.emplace_back("/scans");
+    ts_pages.emplace_back("/stacks");
     ts_pages.emplace_back("/tablets");
     if (!tablet_id.empty()) {
       ts_pages.push_back(strings::Substitute("/transactions?tablet_id=$0",
                                              tablet_id));
     }
-    ts_pages.emplace_back("/maintenance-manager");
-    ts_pages.emplace_back("/mem-trackers");
-    ts_pages.emplace_back("/scans");
 
     // Generate list of urls for each master and tablet server
     for (int i = 0; i < cluster.num_masters(); i++) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/270dd999/src/kudu/server/default_path_handlers.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/default_path_handlers.cc b/src/kudu/server/default_path_handlers.cc
index 8322017..3ecc0fe 100644
--- a/src/kudu/server/default_path_handlers.cc
+++ b/src/kudu/server/default_path_handlers.cc
@@ -18,13 +18,17 @@
 #include "kudu/server/default_path_handlers.h"
 
 #include <sys/stat.h>
+#include <sys/types.h>
 
 #include <cstddef>
 #include <cstdint>
 #include <fstream>
+#include <iterator>
+#include <map>
 #include <memory>
 #include <string>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include <boost/algorithm/string/predicate.hpp>
@@ -44,10 +48,13 @@
 #include "kudu/gutil/strings/human_readable.h"
 #include "kudu/gutil/strings/numbers.h"
 #include "kudu/gutil/strings/split.h"
+#include "kudu/gutil/strings/strip.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/server/pprof_path_handlers.h"
 #include "kudu/server/webserver.h"
+#include "kudu/util/debug-util.h"
 #include "kudu/util/easy_json.h"
+#include "kudu/util/env.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/flags.h"
@@ -55,6 +62,7 @@
 #include "kudu/util/logging.h"
 #include "kudu/util/mem_tracker.h"
 #include "kudu/util/metrics.h"
+#include "kudu/util/monotime.h"
 #include "kudu/util/process_memory.h"
 #include "kudu/util/status.h"
 #include "kudu/util/web_callback_registry.h"
@@ -144,6 +152,87 @@ static void FlagsHandler(const Webserver::WebRequest& req,
             << tags.end_pre_tag;
 }
 
+// Registered to handle "/stacks".
+//
+// Prints out the current stack trace of all threads in the process.
+static void StacksHandler(const Webserver::WebRequest& /*req*/,
+                          Webserver::PrerenderedWebResponse* resp) {
+  std::ostringstream* output = resp->output;
+  vector<pid_t> tids;
+  Status s = ListThreads(&tids);
+  if (!s.ok()) {
+    *output << "Failed to list threads: " << s.ToString();
+    return;
+  }
+  struct Info {
+    pid_t tid;
+    Status status;
+    string thread_name;
+    StackTrace stack;
+  };
+  std::multimap<string, Info> grouped_infos;
+  vector<Info> failed;
+
+  // Capture all the stacks without symbolization initially so that
+  // the stack traces come from as close together in time as possible.
+  //
+  // TODO(todd): would be good to actually send the dump signal to all
+  // threads and then wait for them all to collect their traces, to get
+  // an even tighter snapshot.
+  MonoTime start = MonoTime::Now();
+  for (int i = 0; i < tids.size(); i++) {
+    Info info;
+    info.tid = tids[i];
+
+    // Get the thread's name by reading proc.
+    // TODO(todd): should we have the dumped thread fill in its own name using
+    // prctl to avoid having to open and read /proc? Or maybe we should use the
+    // Kudu ThreadMgr to get the thread names for the cases where we are using
+    // the kudu::Thread wrapper at least.
+    faststring buf;
+    Status s = ReadFileToString(Env::Default(),
+                                Substitute("/proc/self/task/$0/comm", info.tid),
+                                &buf);
+    if (!s.ok()) {
+      info.thread_name = "<unknown name>";
+    }  else {
+      info.thread_name = buf.ToString();
+      StripTrailingNewline(&info.thread_name);
+    }
+
+    info.status = GetThreadStack(info.tid, &info.stack);
+    if (info.status.ok()) {
+      grouped_infos.emplace(info.stack.ToHexString(), std::move(info));
+    } else {
+      failed.emplace_back(std::move(info));
+    }
+  }
+  MonoDelta dur = MonoTime::Now() - start;
+
+  *output << "Collected stacks from " << grouped_infos.size() << " threads in "
+          << dur.ToString() << "\n";
+  if (!failed.empty()) {
+    *output << "Failed to collect stacks from " << failed.size() << " threads "
+            << "(they may have exited while we were iterating over the threads)\n";
+  }
+  *output << "\n";
+  for (auto it = grouped_infos.begin(); it != grouped_infos.end();) {
+    auto end_group = grouped_infos.equal_range(it->first).second;
+    const auto& stack = it->second.stack;
+    int num_in_group = std::distance(it, end_group);
+    if (num_in_group > 1) {
+      *output << num_in_group << " threads with same stack:\n";
+    }
+
+    while (it != end_group) {
+      const auto& info = it->second;
+      *output << "TID " << info.tid << "(" << info.thread_name << "):\n";
+      ++it;
+    }
+    *output << stack.Symbolize() << "\n\n";
+  }
+}
+
 // Registered to handle "/memz", and prints out memory allocation statistics.
 static void MemUsageHandler(const Webserver::WebRequest& req,
                             Webserver::PrerenderedWebResponse* resp) {
@@ -263,6 +352,10 @@ void AddDefaultPathHandlers(Webserver* webserver) {
   webserver->RegisterPathHandler("/config", "Configuration", ConfigurationHandler,
                                   styled, on_nav_bar);
 
+  webserver->RegisterPrerenderedPathHandler("/stacks", "Stacks", StacksHandler,
+                                            /*is_styled=*/false,
+                                            /*is_on_nav_bar=*/false);
+
   AddPprofPathHandlers(webserver);
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/270dd999/src/kudu/util/debug-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/debug-util.cc b/src/kudu/util/debug-util.cc
index bd382a9..c8f077f 100644
--- a/src/kudu/util/debug-util.cc
+++ b/src/kudu/util/debug-util.cc
@@ -310,7 +310,9 @@ string DumpThreadStack(int64_t tid) {
 
 
 Status ListThreads(vector<pid_t> *tids) {
-#if defined(__linux__)
+#ifndef __linux__
+  return Status::NotSupported("unable to list threads on this platform");
+#else
   DIR *dir = opendir("/proc/self/task/");
   if (dir == NULL) {
     return Status::IOError("failed to open task dir", ErrnoToString(errno), errno);
@@ -327,8 +329,8 @@ Status ListThreads(vector<pid_t> *tids) {
     }
   }
   closedir(dir);
-#endif // defined(__linux__)
   return Status::OK();
+#endif // __linux__
 }
 
 string GetStackTrace() {