kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: Fix kudu-ts-cli crash when there is no data in tablet
Date Fri, 02 Sep 2016 21:16:17 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 4d5fb0d3d -> 45523bb7e


Fix kudu-ts-cli crash when there is no data in tablet

NULL pointer was being sent in as an argument to
RowwiseRowBlockPB::Swap routine, fixing the callsite here.

./bin/kudu-ts-cli dump_tablet f32f6e0b1f354643b6b030599a359fa6 --server_address=127.108.26.1:33958
previously segfaulted to:

$0  0x00000000009c9867 in std::swap<int> (__a=@0x7ffc75c76af8, __b=@0x18) at /opt/rh/devtoolset-3/root/usr/include/c++/4.9.2/bits/move.h:176
$1  0x0000000000b5bf1f in kudu::RowwiseRowBlockPB::Swap (this=0x7ffc75c76ae0, other=0x0) at
/data/10/dinesh/kudu/build/debug/src/kudu/common/wire_protocol.pb.cc:1916
$2  0x000000000085de72 in kudu::client::KuduScanBatch::Data::Reset (this=0x7ffc75c76a80, controller=0x7ffc75c76a20,
projection=0x7ffc75c76b40, client_projection=0x7ffc75c76ca0, data=...) at /data/10/dinesh/kudu/src/kudu/client/scanner-internal.cc:484
$3  0x000000000084d5d9 in kudu::tools::TsAdminClient::DumpTablet (this=0x7ffc75c76e20, tablet_id="1f7a6bd64c604f7fada909bd575708cd")
at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:278
$4  0x000000000084f4bd in kudu::tools::TsCliMain (argc=3, argv=0x7ffc75c773f0) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:455
$5  0x0000000000850079 in main (argc=4, argv=0x7ffc75c773e8) at /data/10/dinesh/kudu/src/kudu/tools/ts-cli.cc:492

Change-Id: I8470377400047236127dd409b4825c5c6ea6bbae
Reviewed-on: http://gerrit.cloudera.org:8080/4134
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>


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

Branch: refs/heads/master
Commit: 45523bb7e9a485a60bdd211cb6eba37b6e6c6292
Parents: 4d5fb0d
Author: Dinesh Bhat <dinesh@cloudera.com>
Authored: Fri Aug 26 10:46:35 2016 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Fri Sep 2 21:15:47 2016 +0000

----------------------------------------------------------------------
 src/kudu/tools/kudu-ts-cli-test.cc | 90 +++++++++++++++++++++++++++------
 src/kudu/tools/ts-cli.cc           | 31 ++++++------
 2 files changed, 91 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/45523bb7/src/kudu/tools/kudu-ts-cli-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-ts-cli-test.cc b/src/kudu/tools/kudu-ts-cli-test.cc
index 02d9c2e..6d201a6 100644
--- a/src/kudu/tools/kudu-ts-cli-test.cc
+++ b/src/kudu/tools/kudu-ts-cli-test.cc
@@ -28,6 +28,7 @@
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/util/path_util.h"
 #include "kudu/util/subprocess.h"
+#include "kudu/util/test_macros.h"
 
 using kudu::itest::TabletServerMap;
 using kudu::itest::TServerDetails;
@@ -54,13 +55,12 @@ string KuduTsCliTest::GetTsCliToolPath() const {
   return tool_path;
 }
 
-// Test deleting a tablet.
+// Test deleting a tablet using kudu-ts-cli tool.
 TEST_F(KuduTsCliTest, TestDeleteTablet) {
   MonoDelta timeout = MonoDelta::FromSeconds(30);
-  vector<string> ts_flags, master_flags;
-  ts_flags.push_back("--enable_leader_failure_detection=false");
-  master_flags.push_back("--catalog_manager_wait_for_new_tablets_to_elect_leader=false");
-  NO_FATALS(StartCluster(ts_flags, master_flags));
+  NO_FATALS(StartCluster(
+        {"--enable_leader_failure_detection=false"},
+        {"--catalog_manager_wait_for_new_tablets_to_elect_leader=false"}));
 
   TestWorkload workload(cluster_.get());
   workload.Setup(); // Easy way to create a new tablet.
@@ -76,21 +76,81 @@ TEST_F(KuduTsCliTest, TestDeleteTablet) {
     ASSERT_OK(itest::WaitUntilTabletRunning(ts_map_[cluster_->tablet_server(i)->uuid()],
                                             tablet_id, timeout));
   }
-
-  string exe_path = GetTsCliToolPath();
-  vector<string> argv;
-  argv.push_back(exe_path);
-  argv.push_back("--server_address");
-  argv.push_back(cluster_->tablet_server(0)->bound_rpc_addr().ToString());
-  argv.push_back("delete_tablet");
-  argv.push_back(tablet_id);
-  argv.push_back("Deleting for kudu-ts-cli-test");
-  ASSERT_OK(Subprocess::Call(argv));
+  string out;
+  ASSERT_OK(Subprocess::Call({
+    GetTsCliToolPath(),
+    "--server_address",
+    cluster_->tablet_server(0)->bound_rpc_addr().ToString(),
+    "delete_tablet",
+    tablet_id,
+    "Deleting for kudu-ts-cli-test"
+  }, &out));
+  ASSERT_EQ("", out);
 
   ASSERT_OK(inspect_->WaitForTabletDataStateOnTS(0, tablet_id, { tablet::TABLET_DATA_TOMBSTONED
}));
   TServerDetails* ts = ts_map_[cluster_->tablet_server(0)->uuid()];
   ASSERT_OK(itest::WaitUntilTabletInState(ts, tablet_id, tablet::SHUTDOWN, timeout));
 }
 
+// Test dumping a tablet using kudu-ts-cli tool.
+TEST_F(KuduTsCliTest, TestDumpTablet) {
+  const int kNumRows = 5;
+  MonoDelta timeout = MonoDelta::FromSeconds(30);
+  NO_FATALS(StartCluster({}, {}));
+
+  TestWorkload workload(cluster_.get());
+  workload.set_write_batch_size(1); // One batch is enough to dump some output.
+  workload.Setup();
+
+  vector<tserver::ListTabletsResponsePB::StatusAndSchemaPB> tablets;
+  for (const itest::TabletServerMap::value_type& entry : ts_map_) {
+    TServerDetails* ts = entry.second;
+    ASSERT_OK(itest::WaitForNumTabletsOnTS(ts, 1, timeout, &tablets));
+  }
+  string tablet_id = tablets[0].tablet_status().tablet_id();
+
+  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+    ASSERT_OK(itest::WaitUntilTabletRunning(ts_map_[cluster_->tablet_server(i)->uuid()],
+                                            tablet_id, timeout));
+  }
+
+  string out;
+  // Test for dump_tablet when there is no data in tablet.
+  ASSERT_OK(Subprocess::Call({
+    GetTsCliToolPath(),
+    "--server_address",
+    cluster_->tablet_server(0)->bound_rpc_addr().ToString(),
+    "dump_tablet",
+    tablet_id
+  }, &out));
+  ASSERT_EQ("", out);
+
+  // Insert very little data and dump_tablet again.
+  workload.Start();
+  while (workload.rows_inserted() < kNumRows) {
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  workload.StopAndJoin();
+  ASSERT_OK(WaitForServersToAgree(timeout, ts_map_, tablet_id, workload.batches_completed()));
+  ASSERT_OK(Subprocess::Call({
+    GetTsCliToolPath(),
+    "--server_address",
+    cluster_->tablet_server(0)->bound_rpc_addr().ToString(),
+    "dump_tablet",
+    tablet_id
+  }, &out));
+
+  // Split the output into multiple rows and check format of each row,
+  // and also check total number of rows are at least kNumRows.
+  int nrows = 0;
+  vector<string> rows = strings::Split(out, "\n", strings::SkipEmpty());
+  for (const auto& row : rows) {
+    ASSERT_STR_MATCHES(row, "int32 key=");
+    ASSERT_STR_MATCHES(row, "int32 int_val=");
+    ASSERT_STR_MATCHES(row, "string string_val=hello world");
+    nrows++;
+  }
+  ASSERT_GE(nrows, kNumRows);
+}
 } // namespace tools
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/45523bb7/src/kudu/tools/ts-cli.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ts-cli.cc b/src/kudu/tools/ts-cli.cc
index 80bcda3..f34adbf 100644
--- a/src/kudu/tools/ts-cli.cc
+++ b/src/kudu/tools/ts-cli.cc
@@ -262,8 +262,7 @@ Status TsAdminClient::DumpTablet(const std::string& tablet_id) {
   new_req->set_order_mode(ORDERED);
   new_req->set_read_mode(READ_AT_SNAPSHOT);
 
-  vector<KuduRowResult> rows;
-  while (true) {
+  do {
     RpcController rpc;
     rpc.set_timeout(timeout_);
     RETURN_NOT_OK_PREPEND(ts_proxy_->Scan(req, &resp, &rpc),
@@ -273,28 +272,30 @@ Status TsAdminClient::DumpTablet(const std::string& tablet_id) {
       return Status::IOError("Failed to read: ", resp.error().ShortDebugString());
     }
 
-    rows.clear();
+    // The first response has a scanner ID. We use this for all subsequent
+    // responses.
+    if (resp.has_scanner_id()) {
+      req.set_scanner_id(resp.scanner_id());
+      req.clear_new_scan_request();
+    }
+    req.set_call_seq_id(req.call_seq_id() + 1);
+
+    // Nothing to process from this scan result.
+    if (!resp.has_data()) {
+      continue;
+    }
+
     KuduScanBatch::Data results;
     RETURN_NOT_OK(results.Reset(&rpc,
                                 &schema,
                                 &client_schema,
                                 make_gscoped_ptr(resp.release_data())));
+    vector<KuduRowResult> rows;
     results.ExtractRows(&rows);
     for (const KuduRowResult& r : rows) {
       std::cout << r.ToString() << std::endl;
     }
-
-    // The first response has a scanner ID. We use this for all subsequent
-    // responses.
-    if (resp.has_scanner_id()) {
-      req.set_scanner_id(resp.scanner_id());
-      req.clear_new_scan_request();
-    }
-    req.set_call_seq_id(req.call_seq_id() + 1);
-    if (!resp.has_more_results()) {
-      break;
-    }
-  }
+  } while (resp.has_more_results());
   return Status::OK();
 }
 


Mime
View raw message