[2/3] kudu git commit: web_ui: don't report failed tombstones as live

[mp...@apache.org]

web_ui: don't report failed tombstones as live

Previously, the logic to determine the liveness of a tablet in the web
UI was to check that the replica's state message was exactly
"TABLET_DATA_TOMBSTONED". For failed, tombstoned  replicas, this message
is "FAILED(TABLET_DATA_TOMBSTONED)", and as such, these replicas were
being reported as live, failed tablets (scary!).

This patch adjusts the logic to instead search for
"TABLET_DATA_TOMBSTONED" in the message, which should be relatively
short anyway.

I considered actually using the replicas' internal 'state_' members, but
felt that this is more appropriate considering it'd be debatable how to
report STOPPING or FAILED tablets.

A test is added to tablet_server-test that fails without the fix.

Change-Id: I3529c32296b555d59ddde2593360b3b66081ba4b
Reviewed-on: http://gerrit.cloudera.org:8080/8718
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Mike Percy <mp...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/8739
Reviewed-by: Andrew Wong <aw...@cloudera.com>
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/b2a97b3f
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b2a97b3f
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b2a97b3f

Branch: refs/heads/branch-1.6.x
Commit: b2a97b3f7bf65c2022faa189229ca60a64eceeaf
Parents: 10bca9d
Author: Andrew Wong <aw...@cloudera.com>
Authored: Fri Dec 1 15:14:44 2017 -0800
Committer: Mike Percy <mp...@apache.org>
Committed: Sat Dec 2 05:31:31 2017 +0000

----------------------------------------------------------------------
 src/kudu/tserver/tablet_server-test.cc    | 27 ++++++++++++++++++++++++++
 src/kudu/tserver/tserver_path_handlers.cc |  2 +-
 2 files changed, 28 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a97b3f/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 eb037e0..de498dc 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -29,6 +29,7 @@
 #include <vector>
 
 #include <boost/bind.hpp>
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
@@ -358,6 +359,32 @@ TEST_F(TabletServerTest, TestWebPages) {
 #endif
 }
 
+// Test that tablets that get failed and deleted will eventually show up as
+// failed tombstones on the web UI.
+TEST_F(TabletServerTest, TestFailedTabletsOnWebUI) {
+  scoped_refptr<TabletReplica> replica;
+  TSTabletManager* tablet_manager = mini_server_->server()->tablet_manager();
+  ASSERT_TRUE(tablet_manager->LookupTablet(kTabletId, &replica));
+  replica->SetError(Status::IOError("This error will leave the replica FAILED state at shutdown"));
+  replica->Shutdown();
+  ASSERT_EQ(tablet::FAILED, replica->state());
+
+  // Now delete the tablet and leave it tombstoned, e.g. as if the failed
+  // replica were deleted.
+  TabletServerErrorPB::Code error_code;
+  ASSERT_OK(tablet_manager->DeleteTablet(kTabletId,
+      tablet::TABLET_DATA_TOMBSTONED, boost::none, &error_code));
+
+  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, indicating the
+  // failed tombstone is correctly displayed as a tombstone.
+  ASSERT_STR_CONTAINS(buf.ToString(), "Tombstoned Tablets");
+}
+
 class TabletServerDiskFailureTest : public TabletServerTestBase {
  public:
   virtual void SetUp() override {

http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a97b3f/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 c982d41..3dd602f 100644
--- a/src/kudu/tserver/tserver_path_handlers.cc
+++ b/src/kudu/tserver/tserver_path_handlers.cc
@@ -305,7 +305,7 @@ 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() != "TABLET_DATA_TOMBSTONED") {
+    if (replica->HumanReadableState().find("TABLET_DATA_TOMBSTONED") == string::npos) {
       live_replicas.push_back(replica);
     } else {
       tombstoned_replicas.push_back(replica);