You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2020/03/17 20:00:23 UTC

[kudu] branch master updated: [tools] add test coverage for flags categories in ksck

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 6c10c66  [tools] add test coverage for flags categories in ksck
6c10c66 is described below

commit 6c10c668c28f2127d9000793f41b1cee065efa00
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Mar 17 00:26:47 2020 -0700

    [tools] add test coverage for flags categories in ksck
    
    This patch updates the ksck-test to cover the functionality introduced
    in changelist a9afbce14454f9b03b9cc5a699a09f4f9935da58.
    
    I also updated the output of ksck to unify the naming of tablet servers
    between different parts of the ksck output; one other minor formatting
    change in the description of the --flags_categories_to_check flag.
    
    Change-Id: Ib24b1ee38ffda6b551b1b84fd356a4581e1fe256
    Reviewed-on: http://gerrit.cloudera.org:8080/15458
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/tools/ksck-test.cc | 152 ++++++++++++++++++++++++++++++++++++++++++++
 src/kudu/tools/ksck.cc      |   8 +--
 2 files changed, 156 insertions(+), 4 deletions(-)

diff --git a/src/kudu/tools/ksck-test.cc b/src/kudu/tools/ksck-test.cc
index db4861a..fe03d80 100644
--- a/src/kudu/tools/ksck-test.cc
+++ b/src/kudu/tools/ksck-test.cc
@@ -60,6 +60,7 @@ DECLARE_bool(checksum_scan);
 DECLARE_int32(checksum_idle_timeout_sec);
 DECLARE_int32(max_progress_report_wait_ms);
 DECLARE_string(color);
+DECLARE_string(flags_categories_to_check);
 DECLARE_string(ksck_format);
 DECLARE_uint32(truncate_server_csv_length);
 
@@ -1083,6 +1084,9 @@ TEST_F(KsckTest, TestLeaderMasterUnavailable) {
 }
 
 TEST_F(KsckTest, TestMasterFlagCheck) {
+  // Check for the differences in the 'unusual' flags category.
+  FLAGS_flags_categories_to_check = "unusual";
+
   // Setup flags for each mock master.
   for (int i = 0; i < cluster_->masters().size(); i++) {
     server::GetFlagsResponsePB flags;
@@ -1115,8 +1119,10 @@ TEST_F(KsckTest, TestMasterFlagCheck) {
   }
   ASSERT_OK(ksck_->CheckMasterHealth());
   ASSERT_OK(ksck_->CheckMasterUnusualFlags());
+  ASSERT_OK(ksck_->CheckMasterDivergedFlags());
   ASSERT_OK(ksck_->PrintResults());
   ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Unusual flags for Master:\n"
       "        Flag        | Value |     Tags      |         Master\n"
       "--------------------+-------+---------------+-------------------------\n"
       " experimental_flag  | x     | experimental  | all 3 server(s) checked\n"
@@ -1125,6 +1131,22 @@ TEST_F(KsckTest, TestMasterFlagCheck) {
       " hidden_flag        | 2     | hidden        | master-2\n"
       " hidden_unsafe_flag | 0     | hidden,unsafe | master-0, master-2\n"
       " hidden_unsafe_flag | 1     | hidden,unsafe | master-1");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Some masters have unsafe, experimental, or hidden flags set");
+
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Master:\n"
+      "        Flag        | Value |         Master\n"
+      "--------------------+-------+-------------------------\n"
+      " experimental_flag  | x     | all 3 server(s) checked\n"
+      " hidden_flag        | 0     | master-0\n"
+      " hidden_flag        | 1     | master-1\n"
+      " hidden_flag        | 2     | master-2\n"
+      " hidden_unsafe_flag | 0     | master-0, master-2\n"
+      " hidden_unsafe_flag | 1     | master-1");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Different masters have different settings for same flags "
+      "of checked category 'unusual'");
 }
 
 TEST_F(GetFlagsUnavailableKsckTest, TestMasterFlagsUnavailable) {
@@ -1215,6 +1237,9 @@ TEST_F(KsckTest, TestTserverFlagCheck) {
   // Lower the truncation threshold to test truncation.
   FLAGS_truncate_server_csv_length = 1;
 
+  // Check for the differences in the 'unusual' flags category.
+  FLAGS_flags_categories_to_check = "unusual";
+
   // Setup flags for each mock tablet server.
   int i = 0;
   for (const auto& entry : cluster_->tablet_servers()) {
@@ -1249,8 +1274,10 @@ TEST_F(KsckTest, TestTserverFlagCheck) {
   }
   ASSERT_OK(ksck_->FetchInfoFromTabletServers());
   ASSERT_OK(ksck_->CheckTabletServerUnusualFlags());
+  ASSERT_OK(ksck_->CheckTabletServerDivergedFlags());
   ASSERT_OK(ksck_->PrintResults());
   ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Unusual flags for Tablet Server:\n"
       "        Flag        | Value |     Tags      |         Tablet Server\n"
       "--------------------+-------+---------------+-------------------------------\n"
       " experimental_flag  | x     | experimental  | all 3 server(s) checked\n"
@@ -1259,6 +1286,130 @@ TEST_F(KsckTest, TestTserverFlagCheck) {
       " hidden_flag        | 2     | hidden        | <mock>\n"
       " hidden_unsafe_flag | 0     | hidden,unsafe | <mock>, and 1 other server(s)\n"
       " hidden_unsafe_flag | 1     | hidden,unsafe | <mock>");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Some tablet servers have unsafe, experimental, or hidden flags set");
+
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Tablet Server:\n"
+      "        Flag        | Value |         Tablet Server\n"
+      "--------------------+-------+-------------------------------\n"
+      " experimental_flag  | x     | all 3 server(s) checked\n"
+      " hidden_flag        | 0     | <mock>\n"
+      " hidden_flag        | 1     | <mock>\n"
+      " hidden_flag        | 2     | <mock>\n"
+      " hidden_unsafe_flag | 0     | <mock>, and 1 other server(s)\n"
+      " hidden_unsafe_flag | 1     | <mock>");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Different tablet servers have different settings for same flags "
+      "of checked category 'unusual'");
+}
+
+TEST_F(KsckTest, FlagsCategoriesDifferenceBetweenMastersAndTabletServers) {
+  // Check for the differences in the 'time_source' flags category.
+  FLAGS_flags_categories_to_check = "time_source";
+
+  // Setup flags for mock masters.
+  for (const auto& master : cluster_->masters()) {
+    shared_ptr<MockKsckMaster> m =
+        std::static_pointer_cast<MockKsckMaster>(master);
+    // Set two flags in the 'time_source' category.
+    {
+      server::GetFlagsResponsePB flags;
+      {
+        auto* flag = flags.add_flags();
+        flag->set_name("time_source");
+        flag->set_value("builtin");
+      }
+      {
+        auto* flag = flags.add_flags();
+        flag->set_name("builtin_ntp_servers");
+        flag->set_value("mega.turbo.ntp");
+      }
+      m->flags_by_category_[FlagsCategory::TIME_SOURCE].flags = std::move(flags);
+    }
+
+    // Set a flag unrelated to the checked category.
+    {
+      server::GetFlagsResponsePB flags;
+      {
+        auto* flag = flags.add_flags();
+        flag->set_name("giga");
+        flag->set_value("hertz");
+      }
+      m->flags_by_category_[FlagsCategory::UNUSUAL].flags = std::move(flags);
+    }
+  }
+
+  // Setup flags for mock tablet servers.
+  for (const auto& entry : cluster_->tablet_servers()) {
+    shared_ptr<MockKsckTabletServer> ts =
+        std::static_pointer_cast<MockKsckTabletServer>(entry.second);
+    // Set one flag in the 'time_source' category.
+    {
+      server::GetFlagsResponsePB flags;
+      {
+        auto* flag = flags.add_flags();
+        flag->set_name("time_source");
+        flag->set_value("system");
+      }
+      ts->flags_by_category_[FlagsCategory::TIME_SOURCE].flags = std::move(flags);
+    }
+
+    // Set a flag unrelated to the 'time_source' category.
+    {
+      server::GetFlagsResponsePB flags;
+      {
+        auto* flag = flags.add_flags();
+        flag->set_name("foo");
+        flag->set_value("bar");
+      }
+      ts->flags_by_category_[FlagsCategory::UNUSUAL].flags = std::move(flags);
+    }
+  }
+
+  // Calling CheckMasterHealth() is a prerequisite for calling
+  // CheckMasterUnusualFlags().
+  ASSERT_OK(ksck_->CheckMasterHealth());
+  ASSERT_OK(ksck_->CheckMasterDivergedFlags());
+  // Calling FetchInfoFromTabletServers() is a prerequisite for calling
+  // CheckTabletServerDivergedFlags().
+  ASSERT_OK(ksck_->FetchInfoFromTabletServers());
+  ASSERT_OK(ksck_->CheckTabletServerDivergedFlags());
+  ASSERT_OK(ksck_->CheckDivergedFlags());
+  ASSERT_OK(ksck_->PrintResults());
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Master:\n"
+      "        Flag         |     Value      |         Master\n"
+      "---------------------+----------------+-------------------------\n"
+      " builtin_ntp_servers | mega.turbo.ntp | all 3 server(s) checked\n"
+      " time_source         | builtin        | all 3 server(s) checked\n");
+  ASSERT_STR_NOT_CONTAINS(err_stream_.str(),
+      "Different masters have different settings for same flags "
+      "of checked category 'time_source'");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Master diverging from Tablet Server flags:\n"
+      "        Flag         |     Value      |         Master\n"
+      "---------------------+----------------+-------------------------\n"
+      " builtin_ntp_servers | mega.turbo.ntp | all 3 server(s) checked\n"
+      " time_source         | builtin        | all 3 server(s) checked");
+
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Tablet Server:\n"
+      "    Flag     | Value  |      Tablet Server\n"
+      "-------------+--------+-------------------------\n"
+      " time_source | system | all 3 server(s) checked\n");
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+      "Flags of checked categories for Tablet Server diverging from Master flags:\n"
+      "    Flag     | Value  |      Tablet Server\n"
+      "-------------+--------+-------------------------\n"
+      " time_source | system | all 3 server(s) checked");
+  ASSERT_STR_NOT_CONTAINS(err_stream_.str(),
+      "Different tablet servers have different settings for same flags "
+      "of checked category 'time_source'");
+
+  ASSERT_STR_CONTAINS(err_stream_.str(),
+        "Same flags have different values between masters and tablet servers "
+        "for at least one checked flag category");
 }
 
 TEST_F(GetFlagsUnavailableKsckTest, TestTserverFlagsUnavailable) {
@@ -1741,5 +1892,6 @@ TEST_F(KsckTest, TestSectionFilter) {
     CheckJsonStringVsKsckResults(json_output, ksck_->results(), selected_sections);
   }
 }
+
 } // namespace tools
 } // namespace kudu
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index 6025473..e9208fc 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -65,7 +65,7 @@ DEFINE_string(flags_categories_to_check, STR_FLAGS_CATEGORY_TIME_SOURCE,
               "Comma-separated list of flag categories to check for divergence "
               "across the cluster; default is "
               STR_FLAGS_CATEGORY_TIME_SOURCE "; available categories are "
-              STR_FLAGS_CATEGORY_TIME_SOURCE ","
+              STR_FLAGS_CATEGORY_TIME_SOURCE ", "
               STR_FLAGS_CATEGORY_UNUSUAL ".");
 
 DEFINE_string(ksck_format, "plain_concise",
@@ -689,8 +689,8 @@ Status Ksck::CheckTabletServerDivergedFlags() {
         continue;
       }
       results_.warning_messages.push_back(Status::ConfigurationError(
-          Substitute("Different tservers have different settings for same "
-                     "flags of checked category '$0'",
+          Substitute("Different tablet servers have different settings "
+                     "for same flags of checked category '$0'",
                      FlagsCategoryToString(cat))));
       break;
     }
@@ -742,7 +742,7 @@ Status Ksck::CheckDivergedFlags() {
     }
 
     results_.warning_messages.push_back(Status::ConfigurationError(
-        "Same flags have different values between masters and tservers "
+        "Same flags have different values between masters and tablet servers "
         "for at least one checked flag category"));
   }
   return Status::OK();