You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/05/02 01:29:43 UTC

[2/2] kudu git commit: external mini cluster: spawn perf record for each daemon during Start()

external mini cluster: spawn perf record for each daemon during Start()

I want this for the dense node test, but it's not really possible to do from
outside ExternalMiniCluster without missing out on time spent in Start(),
which I was interested in measuring. So here's a generic approach that can
be used by any itest.

I wasn't sure whether this should be configured via EMC option or gflag. I
ended up with the latter because it's not really something a test needs
programmatic access to; it's just something the dev running the test might
want to enable manually.

I also changed the existing perf calls in full_stack-insert-scan-test to use
the new subprocess custom destroy signal. While there I fixed the handling
of "--call-graph"; passing it without "fp" is a syntax error on both el6.6
and Ubuntu 16.04.

Change-Id: I92079f616648788b461d550057b8e23bc9174b71
Reviewed-on: http://gerrit.cloudera.org:8080/6742
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <da...@gmail.com>


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

Branch: refs/heads/master
Commit: 28869e08618af83261b9a698d9445cb954967771
Parents: a17efaa
Author: Adar Dembo <ad...@cloudera.com>
Authored: Wed Apr 26 21:25:29 2017 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Tue May 2 01:27:56 2017 +0000

----------------------------------------------------------------------
 .../integration-tests/external_mini_cluster.cc  | 36 ++++++++++++++
 .../integration-tests/external_mini_cluster.h   |  5 ++
 .../full_stack-insert-scan-test.cc              | 52 ++++++++------------
 3 files changed, 61 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/28869e08/src/kudu/integration-tests/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.cc b/src/kudu/integration-tests/external_mini_cluster.cc
index 72bae63..d476ef0 100644
--- a/src/kudu/integration-tests/external_mini_cluster.cc
+++ b/src/kudu/integration-tests/external_mini_cluster.cc
@@ -68,6 +68,9 @@ using strings::Substitute;
 
 typedef ListTabletsResponsePB::StatusAndSchemaPB StatusAndSchemaPB;
 
+DEFINE_bool(perf_record, false,
+            "Whether to run \"perf record --call-graph fp\" on each daemon in the cluster");
+
 namespace kudu {
 
 static const char* const kMasterBinaryName = "kudu-master";
@@ -268,6 +271,10 @@ Status ExternalMiniCluster::StartSingleMaster() {
   opts.exe = GetBinaryPath(kMasterBinaryName);
   opts.data_dir = GetDataPath(daemon_id);
   opts.log_dir = GetLogPath(daemon_id);
+  if (FLAGS_perf_record) {
+    opts.perf_record_filename =
+        Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
+  }
   opts.extra_flags = SubstituteInFlags(opts_.extra_master_flags, 0);
   scoped_refptr<ExternalMaster> master = new ExternalMaster(opts);
   if (opts_.enable_kerberos) {
@@ -306,6 +313,10 @@ Status ExternalMiniCluster::StartDistributedMasters() {
     opts.exe = exe;
     opts.data_dir = GetDataPath(daemon_id);
     opts.log_dir = GetLogPath(daemon_id);
+    if (FLAGS_perf_record) {
+      opts.perf_record_filename =
+          Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
+    }
     opts.extra_flags = SubstituteInFlags(flags, i);
 
     scoped_refptr<ExternalMaster> peer = new ExternalMaster(opts, peer_addrs[i]);
@@ -353,6 +364,10 @@ Status ExternalMiniCluster::AddTabletServer() {
   opts.exe = GetBinaryPath(kTabletServerBinaryName);
   opts.data_dir = GetDataPath(daemon_id);
   opts.log_dir = GetLogPath(daemon_id);
+  if (FLAGS_perf_record) {
+    opts.perf_record_filename =
+        Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
+  }
   opts.extra_flags = SubstituteInFlags(opts_.extra_tserver_flags, idx);
 
   scoped_refptr<ExternalTabletServer> ts =
@@ -600,6 +615,7 @@ ExternalDaemon::ExternalDaemon(ExternalDaemonOptions opts)
     : messenger_(std::move(opts.messenger)),
       data_dir_(std::move(opts.data_dir)),
       log_dir_(std::move(opts.log_dir)),
+      perf_record_filename_(std::move(opts.perf_record_filename)),
       logtostderr_(opts.logtostderr),
       exe_(std::move(opts.exe)),
       extra_flags_(std::move(opts.extra_flags)) {}
@@ -690,6 +706,7 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
   // the previous info file if it's there.
   ignore_result(Env::Default()->DeleteFile(info_path));
 
+  // Start the daemon.
   gscoped_ptr<Subprocess> p(new Subprocess(argv));
   p->ShareParentStdout(false);
   p->SetEnvVars(extra_env_);
@@ -700,6 +717,22 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
   RETURN_NOT_OK_PREPEND(p->Start(),
                         Substitute("Failed to start subprocess $0", exe_));
 
+  // If requested, start a monitoring subprocess.
+  unique_ptr<Subprocess> perf_record;
+  if (!perf_record_filename_.empty()) {
+    perf_record.reset(new Subprocess({
+      "perf",
+      "record",
+      "--call-graph",
+      "fp",
+      "-o",
+      perf_record_filename_,
+      Substitute("--pid=$0", p->pid())
+    }, SIGINT));
+    RETURN_NOT_OK_PREPEND(perf_record->Start(),
+                          "Could not start perf record subprocess");
+  }
+
   // The process is now starting -- wait for the bound port info to show up.
   Stopwatch sw;
   sw.start();
@@ -721,6 +754,7 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
     // and exit as if it had succeeded.
     if (WIFEXITED(wait_status) && WEXITSTATUS(wait_status) == fault_injection::kExitStatus) {
       process_.swap(p);
+      perf_record_process_.swap(perf_record);
       return Status::OK();
     }
 
@@ -744,6 +778,7 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
   VLOG(1) << exe_ << " instance information:\n" << SecureDebugString(*status_);
 
   process_.swap(p);
+  perf_record_process_.swap(perf_record);
   return Status::OK();
 }
 
@@ -866,6 +901,7 @@ void ExternalDaemon::Shutdown() {
   WARN_NOT_OK(process_->Wait(), "Waiting on " + exe_);
   paused_ = false;
   process_.reset();
+  perf_record_process_.reset();
 }
 
 void ExternalDaemon::FlushCoverage() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/28869e08/src/kudu/integration-tests/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.h b/src/kudu/integration-tests/external_mini_cluster.h
index b910d35..c3d7106 100644
--- a/src/kudu/integration-tests/external_mini_cluster.h
+++ b/src/kudu/integration-tests/external_mini_cluster.h
@@ -43,6 +43,7 @@ class HostPort;
 class MetricPrototype;
 class MetricEntityPrototype;
 class NodeInstancePB;
+class ScopedSubprocess;
 class Sockaddr;
 class Subprocess;
 
@@ -326,6 +327,7 @@ struct ExternalDaemonOptions {
   std::string exe;
   std::string data_dir;
   std::string log_dir;
+  std::string perf_record_filename;
   std::vector<std::string> extra_flags;
 };
 
@@ -456,6 +458,7 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
   const std::shared_ptr<rpc::Messenger> messenger_;
   const std::string data_dir_;
   const std::string log_dir_;
+  const std::string perf_record_filename_;
   const bool logtostderr_;
   std::string exe_;
   std::vector<std::string> extra_flags_;
@@ -464,6 +467,8 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
   gscoped_ptr<Subprocess> process_;
   bool paused_ = false;
 
+  std::unique_ptr<Subprocess> perf_record_process_;
+
   gscoped_ptr<server::ServerStatusPB> status_;
   std::string rpc_bind_address_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/28869e08/src/kudu/integration-tests/full_stack-insert-scan-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/full_stack-insert-scan-test.cc b/src/kudu/integration-tests/full_stack-insert-scan-test.cc
index cf83dba..eab7aff 100644
--- a/src/kudu/integration-tests/full_stack-insert-scan-test.cc
+++ b/src/kudu/integration-tests/full_stack-insert-scan-test.cc
@@ -15,15 +15,17 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include <signal.h>
+
 #include <cmath>
 #include <cstdlib>
-#include <gflags/gflags.h>
-#include <glog/logging.h>
 #include <memory>
-#include <signal.h>
 #include <string>
 #include <vector>
 
+#include <gflags/gflags.h>
+#include <glog/logging.h>
+
 #include "kudu/client/callbacks.h"
 #include "kudu/client/client.h"
 #include "kudu/client/client-test-util.h"
@@ -67,13 +69,15 @@ DEFINE_int32(rows_per_batch, -1, "Number of rows per client batch");
 // Perf-related FLAGS_perf_stat
 DEFINE_bool(perf_record_scan, false, "Call \"perf record --call-graph\" "
             "for the duration of the scan, disabled by default");
-DEFINE_bool(perf_stat_scan, false, "Print \"perf stat\" results during"
+DEFINE_bool(perf_record_scan_callgraph, false,
+            "Only applicable with --perf_record_scan, provides argument "
+            "\"--call-graph fp\"");
+DEFINE_bool(perf_stat_scan, false, "Print \"perf stat\" results during "
             "scan to stdout, disabled by default");
-DEFINE_bool(perf_fp_flag, false, "Only applicable with --perf_record_scan,"
-            " provides argument \"fp\" to the --call-graph flag");
 DECLARE_bool(enable_maintenance_manager);
 
 using std::string;
+using std::unique_ptr;
 using std::vector;
 
 namespace kudu {
@@ -215,33 +219,20 @@ class FullStackInsertScanTest : public KuduTest {
 
 namespace {
 
-gscoped_ptr<Subprocess> MakePerfStat() {
-  if (!FLAGS_perf_stat_scan) return gscoped_ptr<Subprocess>();
+unique_ptr<Subprocess> MakePerfStat() {
+  if (!FLAGS_perf_stat_scan) return unique_ptr<Subprocess>(nullptr);
   // No output flag for perf-stat 2.x, just print to output
   string cmd = Substitute("perf stat --pid=$0", getpid());
   LOG(INFO) << "Calling: \"" << cmd << "\"";
-  return gscoped_ptr<Subprocess>(new Subprocess(Split(cmd, " ")));
+  return unique_ptr<Subprocess>(new Subprocess(Split(cmd, " "), SIGINT));
 }
 
-gscoped_ptr<Subprocess> MakePerfRecord() {
-  if (!FLAGS_perf_record_scan) return gscoped_ptr<Subprocess>();
-  string cmd = Substitute("perf record --pid=$0 --call-graph", getpid());
-  if (FLAGS_perf_fp_flag) cmd += " fp";
+unique_ptr<Subprocess> MakePerfRecord() {
+  if (!FLAGS_perf_record_scan) return unique_ptr<Subprocess>(nullptr);
+  string cmd = Substitute("perf record --pid=$0", getpid());
+  if (FLAGS_perf_record_scan_callgraph) cmd += " --call-graph fp";
   LOG(INFO) << "Calling: \"" << cmd << "\"";
-  return gscoped_ptr<Subprocess>(new Subprocess(Split(cmd, " ")));
-}
-
-void InterruptNotNull(gscoped_ptr<Subprocess> sub) {
-  if (!sub) return;
-
-  ASSERT_OK(sub->Kill(SIGINT));
-  ASSERT_OK(sub->Wait());
-  int exit_status;
-  string exit_info_str;
-  ASSERT_OK(sub->GetExitStatus(&exit_status, &exit_info_str));
-  if (exit_status != 0) {
-    LOG(WARNING) << exit_info_str;
-  }
+  return unique_ptr<Subprocess>(new Subprocess(Split(cmd, " "), SIGINT));
 }
 
 // If key is approximately at an even multiple of 1/10 of the way between
@@ -312,11 +303,11 @@ void FullStackInsertScanTest::DoTestScans() {
   }
   LOG(INFO) << "Doing test scans on table of " << kNumRows << " rows.";
 
-  gscoped_ptr<Subprocess> stat = MakePerfStat();
+  unique_ptr<Subprocess> stat = MakePerfRecord();
   if (stat) {
     ASSERT_OK(stat->Start());
   }
-  gscoped_ptr<Subprocess> record = MakePerfRecord();
+  unique_ptr<Subprocess> record = MakePerfStat();
   if (record) {
     ASSERT_OK(record->Start());
   }
@@ -327,9 +318,6 @@ void FullStackInsertScanTest::DoTestScans() {
   NO_FATALS(ScanProjection(StringColumnNames(), "String projection, 1 col"));
   NO_FATALS(ScanProjection(Int32ColumnNames(), "Int32 projection, 4 col"));
   NO_FATALS(ScanProjection(Int64ColumnNames(), "Int64 projection, 4 col"));
-
-  NO_FATALS(InterruptNotNull(std::move(record)));
-  NO_FATALS(InterruptNotNull(std::move(stat)));
 }
 
 void FullStackInsertScanTest::FlushToDisk() {