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 <[email protected]> Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> 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 <[email protected]> Authored: Fri Aug 26 10:46:35 2016 -0700 Committer: Adar Dembo <[email protected]> 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(); }
