kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [1/3] kudu git commit: [webui] Make tombstone tablet info less confusing
Date Thu, 11 Jan 2018 22:16:23 GMT
Repository: kudu
Updated Branches:
  refs/heads/master f2adee004 -> 56107ac80


[webui] Make tombstone tablet info less confusing

Previously, when a tombstone tablet was reloaded at server startup,
the last status message was "Tablet initializing...". This was
confusing as it set the expectation that something more was going
to happen to the tombstoned tablet. The message is now simply
"Tombstoned".

Also, now that tombstoned tablets can vote, they retain cmeta
despite not participating in non-election Raft operations.
Their list of peers is not updated and not usually relevant. It
might be confusing to see it on the web ui. This patch suppresses
it.

Change-Id: I5c879cc7ff634e5b434fc33374d3010cf1f262cb
Reviewed-on: http://gerrit.cloudera.org:8080/8981
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


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

Branch: refs/heads/master
Commit: 1cf30effc13827ec4f2e739e7ca3afdbc0051437
Parents: f2adee0
Author: Will Berkeley <wdberkeley@apache.org>
Authored: Tue Jan 9 11:23:09 2018 -0800
Committer: Will Berkeley <wdberkeley@gmail.com>
Committed: Thu Jan 11 02:22:40 2018 +0000

----------------------------------------------------------------------
 src/kudu/tablet/tablet_replica.h          |  5 ++++
 src/kudu/tserver/tablet_server-test.cc    | 33 ++++++++++++++++++++++++++
 src/kudu/tserver/ts_tablet_manager.cc     |  1 +
 src/kudu/tserver/tserver_path_handlers.cc | 23 +++++++++++++-----
 4 files changed, 56 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1cf30eff/src/kudu/tablet/tablet_replica.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_replica.h b/src/kudu/tablet/tablet_replica.h
index bf3476d..253e9c0 100644
--- a/src/kudu/tablet/tablet_replica.h
+++ b/src/kudu/tablet/tablet_replica.h
@@ -187,6 +187,11 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
 
   std::string StateName() const;
 
+  const TabletDataState data_state() const {
+    std::lock_guard<simple_spinlock> lock(lock_);
+    return meta_->tablet_data_state();
+  }
+
   // Returns the current Raft configuration.
   const consensus::RaftConfigPB RaftConfig() const;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/1cf30eff/src/kudu/tserver/tablet_server-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_server-test.cc b/src/kudu/tserver/tablet_server-test.cc
index de498dc..411b982 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -385,6 +385,39 @@ TEST_F(TabletServerTest, TestFailedTabletsOnWebUI) {
   ASSERT_STR_CONTAINS(buf.ToString(), "Tombstoned Tablets");
 }
 
+// Test that tombstoned tablets are displayed correctly in the web ui:
+// - After restart, status message of "Tombstoned" instead of "Tablet initializing...".
+// - No consensus configuration.
+TEST_F(TabletServerTest, TestTombstonedTabletOnWebUI) {
+  TSTabletManager* tablet_manager = mini_server_->server()->tablet_manager();
+  TabletServerErrorPB::Code error_code;
+  ASSERT_OK(tablet_manager->DeleteTablet(kTabletId,
+                                         tablet::TABLET_DATA_TOMBSTONED, boost::none, &error_code));
+
+  // Restart the server. We drop the tablet_replica_ reference since it becomes
+  // invalid when the server shuts down.
+  tablet_replica_.reset();
+  mini_server_->Shutdown();
+  ASSERT_OK(mini_server_->Restart());
+  ASSERT_OK(mini_server_->WaitStarted());
+
+  EasyCurl c;
+  faststring buf;
+  const string addr = mini_server_->bound_http_addr().ToString();
+  ASSERT_OK(c.FetchURL(Substitute("http://$0/tablets", addr), &buf));
+
+  // Ensure the html contains the "Tombstoned Tablets" header and
+  // a table entry with the proper status message.
+  string s = buf.ToString();
+  ASSERT_STR_CONTAINS(s, "Tombstoned Tablets");
+  ASSERT_STR_CONTAINS(s, "<td>Tombstoned</td>");
+  ASSERT_STR_NOT_CONTAINS(s, "<td>Tablet initializing...</td>");
+
+  // Since the consensus config shouldn't be displayed, the html should not
+  // contain the server's RPC address.
+  ASSERT_STR_NOT_CONTAINS(s, mini_server_->bound_rpc_addr().ToString());
+}
+
 class TabletServerDiskFailureTest : public TabletServerTestBase {
  public:
   virtual void SetUp() override {

http://git-wip-us.apache.org/repos/asf/kudu/blob/1cf30eff/src/kudu/tserver/ts_tablet_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index bef2d9d..a3c2887 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -1256,6 +1256,7 @@ Status TSTabletManager::HandleNonReadyTabletOnStartup(const scoped_refptr<Tablet
   if (data_state == TABLET_DATA_TOMBSTONED) {
     scoped_refptr<TabletReplica> dummy;
     RETURN_NOT_OK(CreateAndRegisterTabletReplica(meta, NEW_REPLICA, &dummy));
+    dummy->SetStatusMessage("Tombstoned");
   }
 
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/1cf30eff/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 7cfc935..19b8bb6 100644
--- a/src/kudu/tserver/tserver_path_handlers.cc
+++ b/src/kudu/tserver/tserver_path_handlers.cc
@@ -207,6 +207,10 @@ string TabletLink(const string& id) {
                     EscapeForHtmlToString(id));
 }
 
+bool IsTombstoned(const scoped_refptr<TabletReplica>& replica) {
+  return replica->data_state() == tablet::TABLET_DATA_TOMBSTONED;
+}
+
 } // anonymous namespace
 
 void TabletServerPathHandlers::HandleTabletsPage(const Webserver::WebRequest& /*req*/,
@@ -282,7 +286,12 @@ void TabletServerPathHandlers::HandleTabletsPage(const Webserver::WebRequest&
/*
                                  .PartitionDebugString(replica->tablet_metadata()->partition(),
                                                        replica->tablet_metadata()->schema());
 
+      // We don't show the config if it's a tombstone because it's misleading.
       shared_ptr<consensus::RaftConsensus> consensus = replica->shared_consensus();
+      string consensus_state_html =
+          consensus && !IsTombstoned(replica) ? ConsensusStatePBToHtml(consensus->ConsensusState())
+                                              : "";
+
       *output << Substitute(
           // Table name, tablet id, partition
           "<tr><td>$0</td><td>$1</td><td>$2</td>"
@@ -292,7 +301,7 @@ void TabletServerPathHandlers::HandleTabletsPage(const Webserver::WebRequest&
/*
           tablet_id_or_link, // $1
           EscapeForHtmlToString(partition), // $2
           EscapeForHtmlToString(replica->HumanReadableState()), mem_bytes, n_bytes, //
$3, $4, $5
-          consensus ? ConsensusStatePBToHtml(consensus->ConsensusState()) : "", // $6
+          consensus_state_html, // $6
           EscapeForHtmlToString(status.last_status())); // $7
     }
     *output << "<tbody></table>\n</div>\n";
@@ -301,10 +310,10 @@ void TabletServerPathHandlers::HandleTabletsPage(const Webserver::WebRequest&
/*
   vector<scoped_refptr<TabletReplica>> live_replicas;
   vector<scoped_refptr<TabletReplica>> tombstoned_replicas;
   for (const scoped_refptr<TabletReplica>& replica : replicas) {
-    if (replica->HumanReadableState().find("TABLET_DATA_TOMBSTONED") == string::npos)
{
-      live_replicas.push_back(replica);
-    } else {
+    if (IsTombstoned(replica)) {
       tombstoned_replicas.push_back(replica);
+    } else {
+      live_replicas.push_back(replica);
     }
   }
 
@@ -314,8 +323,10 @@ void TabletServerPathHandlers::HandleTabletsPage(const Webserver::WebRequest&
/*
   }
   if (!tombstoned_replicas.empty()) {
     *output << "<h3>Tombstoned Tablets</h3>\n";
-    *output << "<p><small>Tombstoned tablets are tablets that previously
"
-               "stored a replica on this server.</small></p>";
+    *output << "<p><small>Tombstone tablets are necessary for correct operation
"
+               "of Kudu. These tablets have had all of their data removed from "
+               "disk and do not consume significant resources, and must not be "
+               "deleted.</small></p>";
     generate_table(tombstoned_replicas, output);
   }
 }


Mime
View raw message