[kudu] 01/02: KUDU-2680: ignore tombstones when updating dirs

[mp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mpercy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit caf8f7dce0c8d3bfd5761c7d5c59e25e64f9352c
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Tue Jan 29 23:31:50 2019 -0800

    KUDU-2680: ignore tombstones when updating dirs
    
    Previously, when run without the --force configuration, the update_dirs
    tool would fail upon meeting a tombstoned tablet. This is because
    tombstones don't persist data dir groups, nor do they create them
    in-memory upon loading; and the update_dirs tool uses the existence of
    an in-memory data dir group to determine whether all tablets would be
    considered healthy while staging a new directory configuration.
    
    Since we don't expect tombstones to have data dir groups anyway, we
    shouldn't be gating update_dirs on them not having data dir groups.
    
    Change-Id: If39ec768e9328571adc3ed0ef69c6a9309f27989
    Reviewed-on: http://gerrit.cloudera.org:8080/12313
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/tools/kudu-tool-test.cc | 36 ++++++++++++++++++++++++++++++++++++
 src/kudu/tools/tool_action_fs.cc | 14 +++++++++++---
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 9838620..a5f02ea 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -178,6 +178,7 @@ using kudu::tserver::DeleteTabletRequestPB;
 using kudu::tserver::DeleteTabletResponsePB;
 using kudu::tserver::ListTabletsResponsePB;
 using kudu::tserver::MiniTabletServer;
+using kudu::tserver::TabletServerErrorPB;
 using kudu::tserver::WriteRequestPB;
 using std::back_inserter;
 using std::copy;
@@ -3512,6 +3513,41 @@ TEST_F(ToolTest, TestFsSwappingDirectoriesFailsGracefully) {
       wal_root, JoinStrings(new_data_roots, ","))));
 }
 
+TEST_F(ToolTest, TestFsRemoveDataDirWithTombstone) {
+  if (FLAGS_block_manager == "file") {
+    LOG(INFO) << "Skipping test, only log block manager is supported";
+    return;
+  }
+
+  // Start a cluster whose tserver has multiple data directories and create a
+  // tablet on it.
+  InternalMiniClusterOptions opts;
+  opts.num_data_dirs = 2;
+  NO_FATALS(StartMiniCluster(std::move(opts)));
+  NO_FATALS(CreateTableWithFlushedData("tablename", mini_cluster_.get()));
+
+  // Tombstone the tablet.
+  MiniTabletServer* mts = mini_cluster_->mini_tablet_server(0);
+  vector<string> tablet_ids = mts->ListTablets();
+  ASSERT_EQ(1, tablet_ids.size());
+  TabletServerErrorPB::Code error;
+  ASSERT_OK(mts->server()->tablet_manager()->DeleteTablet(
+      tablet_ids[0], TabletDataState::TABLET_DATA_TOMBSTONED, boost::none, &error));
+
+  // Set things up so we can restart with one fewer directory.
+  string data_root = mts->options()->fs_opts.data_roots[0];
+  mts->options()->fs_opts.data_roots = { data_root };
+  mts->Shutdown();
+  // KUDU-2680: tombstones shouldn't prevent us from removing a directory.
+  NO_FATALS(RunActionStdoutNone(Substitute(
+      "fs update_dirs --fs_wal_dir=$0 --fs_data_dirs=$1",
+      mts->options()->fs_opts.wal_root, data_root)));
+
+  ASSERT_OK(mts->Start());
+  ASSERT_OK(mts->WaitStarted());
+  ASSERT_EQ(0, mts->server()->tablet_manager()->GetNumLiveTablets());
+}
+
 TEST_F(ToolTest, TestFsAddRemoveDataDirEndToEnd) {
   const string kTableFoo = "foo";
   const string kTableBar = "bar";
diff --git a/src/kudu/tools/tool_action_fs.cc b/src/kudu/tools/tool_action_fs.cc
index 88072e7..04a8c0c 100644
--- a/src/kudu/tools/tool_action_fs.cc
+++ b/src/kudu/tools/tool_action_fs.cc
@@ -61,6 +61,7 @@
 #include "kudu/tablet/delta_stats.h"
 #include "kudu/tablet/deltafile.h"
 #include "kudu/tablet/diskrowset.h"
+#include "kudu/tablet/metadata.pb.h"
 #include "kudu/tablet/rowset_metadata.h"
 #include "kudu/tablet/tablet.pb.h"
 #include "kudu/tablet/tablet_metadata.h"
@@ -119,6 +120,7 @@ using std::unordered_map;
 using std::vector;
 using strings::Substitute;
 using tablet::RowSetMetadata;
+using tablet::TabletDataState;
 using tablet::TabletMetadata;
 
 namespace {
@@ -331,9 +333,15 @@ Status CheckForTabletsThatWillFailWithUpdate() {
     scoped_refptr<TabletMetadata> meta;
     RETURN_NOT_OK(TabletMetadata::Load(&fs, t, &meta));
     DataDirGroupPB group;
-    RETURN_NOT_OK_PREPEND(
-        fs.dd_manager()->GetDataDirGroupPB(t, &group),
-        "at least one tablet is configured to use removed data directory. "
+    Status s = fs.dd_manager()->GetDataDirGroupPB(t, &group);
+    if (meta->tablet_data_state() == TabletDataState::TABLET_DATA_TOMBSTONED) {
+      // If we just loaded a tombstoned tablet, there should be no in-memory
+      // data dir group for the tablet, and the staged directory config won't
+      // affect this tablet.
+      DCHECK(s.IsNotFound()) << s.ToString();
+      continue;
+    }
+    RETURN_NOT_OK_PREPEND(s, "at least one tablet is configured to use removed data directory. "
         "Retry with --force to override this");
   }
   return Status::OK();