Repository: kudu Updated Branches: refs/heads/master c553a9a59 -> b39491de6
KUDU-1545. Fix crash when visiting tablet page for tombstoned tablet This page was previously accessing peer->consensus() which could be NULL in a tombstoned tablet. Change-Id: I7d53d2080c476fe02e2c4ab5482fe80387199377 Reviewed-on: http://gerrit.cloudera.org:8080/5280 Reviewed-by: Dan Burkert <[email protected]> Reviewed-by: Will Berkeley <[email protected]> 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/885d2e90 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/885d2e90 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/885d2e90 Branch: refs/heads/master Commit: 885d2e90a8c9e5ceb8ff330478a8b08c7ba3b2a0 Parents: c553a9a Author: Todd Lipcon <[email protected]> Authored: Wed Nov 30 12:04:09 2016 -0800 Committer: Todd Lipcon <[email protected]> Committed: Wed Nov 30 20:50:26 2016 +0000 ---------------------------------------------------------------------- src/kudu/integration-tests/delete_table-test.cc | 45 +++++++++++++++++++- src/kudu/tserver/tserver-path-handlers.cc | 9 +++- 2 files changed, 50 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/885d2e90/src/kudu/integration-tests/delete_table-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/delete_table-test.cc b/src/kudu/integration-tests/delete_table-test.cc index 08b48c3..33d2e51 100644 --- a/src/kudu/integration-tests/delete_table-test.cc +++ b/src/kudu/integration-tests/delete_table-test.cc @@ -269,8 +269,8 @@ TEST_F(DeleteTableTest, TestDeleteEmptyTable) { ASSERT_EQ("{\"tables\":[],\"tablets\":[]}", entities_buf.ToString()); } -// Test that a DeleteTable RPC is rejected without a matching destination UUID. -TEST_F(DeleteTableTest, TestDeleteTableDestUuidValidation) { +// Test that a DeleteTablet RPC is rejected without a matching destination UUID. +TEST_F(DeleteTableTest, TestDeleteTabletDestUuidValidation) { NO_FATALS(StartCluster()); // Create a table on the cluster. We're just using TestWorkload // as a convenient way to create it. @@ -970,6 +970,47 @@ TEST_F(DeleteTableTest, TestFDsNotLeakedOnTabletTombstone) { ASSERT_EQ(0, PrintOpenTabletFiles(ets->pid(), tablet_id)); } +// Regression test for KUDU-1545: crash when visiting the tablet page for a +// tombstoned tablet. +TEST_F(DeleteTableTest, TestWebPageForTombstonedTablet) { + const MonoDelta timeout = MonoDelta::FromSeconds(30); + + NO_FATALS(StartCluster({}, {}, 1)); + + // Create the table. + TestWorkload workload(cluster_.get()); + workload.set_num_replicas(1); + workload.Setup(); + + // Figure out the tablet id of the created tablet. + vector<ListTabletsResponsePB::StatusAndSchemaPB> tablets; + ASSERT_OK(WaitForNumTabletsOnTS(ts_map_.begin()->second, 1, timeout, &tablets)); + const string& tablet_id = tablets[0].tablet_status().tablet_id(); + + // Tombstone the tablet. + ExternalTabletServer* ets = cluster_->tablet_server(0); + ASSERT_OK(itest::DeleteTablet(ts_map_[ets->uuid()], + tablet_id, TABLET_DATA_TOMBSTONED, boost::none, timeout)); + + // Check the various web pages associated with the tablet, ensuring + // they don't crash and at least have the tablet ID within them. + EasyCurl c; + const auto& pages = { "tablet", + "tablet-rowsetlayout-svg", + "tablet-consensus-status", + "log-anchors" }; + for (const auto& page : pages) { + faststring buf; + ASSERT_OK(c.FetchURL(Substitute( + "http://$0/$1?id=$2";, + cluster_->tablet_server(0)->bound_http_hostport().ToString(), + page, + tablet_id), &buf)); + ASSERT_STR_CONTAINS(buf.ToString(), tablet_id); + } +} + + TEST_F(DeleteTableTest, TestUnknownTabletsAreNotDeleted) { // Speed up heartbeating so that the unknown tablet is detected faster. vector<string> extra_ts_flags = { "--heartbeat_interval_ms=10" }; http://git-wip-us.apache.org/repos/asf/kudu/blob/885d2e90/src/kudu/tserver/tserver-path-handlers.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tserver/tserver-path-handlers.cc b/src/kudu/tserver/tserver-path-handlers.cc index dc671cc..95e42c8 100644 --- a/src/kudu/tserver/tserver-path-handlers.cc +++ b/src/kudu/tserver/tserver-path-handlers.cc @@ -343,10 +343,15 @@ void TabletServerPathHandlers::HandleTabletPage(const Webserver::WebRequest& req if (!LoadTablet(tserver_, req, &tablet_id, &peer, output)) return; string table_name = peer->tablet_metadata()->table_name(); - RaftPeerPB::Role role = peer->consensus()->role(); + RaftPeerPB::Role role = RaftPeerPB::UNKNOWN_ROLE; + auto consensus = peer->consensus(); + if (consensus) { + role = consensus->role(); + } *output << "<h1>Tablet " << EscapeForHtmlToString(tablet_id) - << " (" << RaftPeerPB::Role_Name(role) << ")</h1>\n"; + << " (" << peer->HumanReadableState() + << "/" << RaftPeerPB::Role_Name(role) << ")</h1>\n"; *output << "<h3>Table " << EscapeForHtmlToString(table_name) << "</h3>"; // Output schema in tabular format.
