kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aw...@apache.org
Subject [kudu] branch master updated: [maintenance] Add extra config for maintenance manager task priority
Date Tue, 16 Jul 2019 21:34:09 GMT
This is an automated email from the ASF dual-hosted git repository.

awong 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 c7bf122  [maintenance] Add extra config for maintenance manager task priority
c7bf122 is described below

commit c7bf122f3abcb48f6c0ab52ba1b24f1c64174f7a
Author: Yingchun Lai <405403881@qq.com>
AuthorDate: Wed Jul 10 16:27:26 2019 +0800

    [maintenance] Add extra config for maintenance manager task priority
    
    While set and update priorities for multiply tables by GFlags is a
    compromise, now we can improve it by extra config of tables.
    
    Change-Id: I966ee626ef85ce56ba4517b9e494f3ac5b044867
    Reviewed-on: http://gerrit.cloudera.org:8080/13659
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Tested-by: Yingchun Lai <405403881@qq.com>
---
 src/kudu/common/common.proto                   |  5 +++
 src/kudu/common/wire_protocol.cc               | 24 +++++++++--
 src/kudu/common/wire_protocol.h                |  3 ++
 src/kudu/integration-tests/alter_table-test.cc | 30 +++++++++++++
 src/kudu/tablet/tablet.h                       |  4 +-
 src/kudu/tablet/tablet_mm_ops.cc               | 12 +++++-
 src/kudu/tablet/tablet_mm_ops.h                |  2 +-
 src/kudu/tablet/tablet_replica.h               |  3 +-
 src/kudu/tablet/tablet_replica_mm_ops.cc       | 12 +++++-
 src/kudu/tablet/tablet_replica_mm_ops.h        |  2 +-
 src/kudu/util/maintenance_manager-test.cc      | 59 ++++++++++----------------
 src/kudu/util/maintenance_manager.cc           | 44 +++----------------
 src/kudu/util/maintenance_manager.h            | 12 ++----
 13 files changed, 116 insertions(+), 96 deletions(-)

diff --git a/src/kudu/common/common.proto b/src/kudu/common/common.proto
index a303803..a3ffd24 100644
--- a/src/kudu/common/common.proto
+++ b/src/kudu/common/common.proto
@@ -445,4 +445,9 @@ message TableExtraConfigPB {
   // backups. Reads initiated at a snapshot that is older than this
   // age will be rejected. Equivalent to --tablet_history_max_age_sec.
   optional int32 history_max_age_sec = 1;
+
+  // Priority level of a table for maintenance, it will be clamped into
+  // range [-FLAGS_max_priority_range, FLAGS_max_priority_range] when
+  // calculate maintenance priority score.
+  optional int32 maintenance_priority = 2;
 }
diff --git a/src/kudu/common/wire_protocol.cc b/src/kudu/common/wire_protocol.cc
index d40f813..9350231 100644
--- a/src/kudu/common/wire_protocol.cc
+++ b/src/kudu/common/wire_protocol.cc
@@ -667,6 +667,7 @@ Status ColumnPredicateFromPB(const Schema& schema,
 }
 
 const char kTableHistoryMaxAgeSec[] = "kudu.table.history_max_age_sec";
+const char kTableMaintenancePriority[] = "kudu.table.maintenance_priority";
 Status ExtraConfigPBToMap(const TableExtraConfigPB& pb, map<string, string>* configs)
{
   Map<string, string> tmp;
   RETURN_NOT_OK(ExtraConfigPBToPBMap(pb, &tmp));
@@ -675,6 +676,14 @@ Status ExtraConfigPBToMap(const TableExtraConfigPB& pb, map<string,
string>* con
   return Status::OK();
 }
 
+Status ParseInt32Config(const string& name, const string& value, int32_t* result)
{
+  CHECK(result);
+  if (!safe_strto32(value, result)) {
+    return Status::InvalidArgument(Substitute("unable to parse $0", name), value);
+  }
+  return Status::OK();
+}
+
 Status ExtraConfigPBFromPBMap(const Map<string, string>& configs, TableExtraConfigPB*
pb) {
   TableExtraConfigPB result;
   for (const auto& config : configs) {
@@ -683,11 +692,15 @@ Status ExtraConfigPBFromPBMap(const Map<string, string>& configs,
TableExtraConf
     if (name == kTableHistoryMaxAgeSec) {
       if (!value.empty()) {
         int32_t history_max_age_sec;
-        if (!safe_strto32(value, &history_max_age_sec)) {
-          return Status::InvalidArgument(Substitute("Unable to parse $0", name), value);
-        }
+        RETURN_NOT_OK(ParseInt32Config(name, value, &history_max_age_sec));
         result.set_history_max_age_sec(history_max_age_sec);
       }
+    } else if (name == kTableMaintenancePriority) {
+      if (!value.empty()) {
+        int32_t maintenance_priority;
+        RETURN_NOT_OK(ParseInt32Config(name, value, &maintenance_priority));
+        result.set_maintenance_priority(maintenance_priority);
+      }
     } else {
       LOG(WARNING) << "Unknown extra configuration property: " << name;
     }
@@ -701,6 +714,9 @@ Status ExtraConfigPBToPBMap(const TableExtraConfigPB& pb, Map<string,
string>* c
   if (pb.has_history_max_age_sec()) {
     result[kTableHistoryMaxAgeSec] = std::to_string(pb.history_max_age_sec());
   }
+  if (pb.has_maintenance_priority()) {
+    result[kTableMaintenancePriority] = std::to_string(pb.maintenance_priority());
+  }
   *configs = std::move(result);
   return Status::OK();
 }
@@ -1024,7 +1040,7 @@ void SerializeRowBlock(const RowBlock& block,
   rowblock_pb->set_num_rows(rowblock_pb->num_rows() + num_rows);
 }
 
-std::string StartTimeToString(const ServerRegistrationPB& reg) {
+string StartTimeToString(const ServerRegistrationPB& reg) {
   string start_time;
   if (reg.has_start_time()) {
     // Convert epoch time to localtime.
diff --git a/src/kudu/common/wire_protocol.h b/src/kudu/common/wire_protocol.h
index e29c568..679dc90 100644
--- a/src/kudu/common/wire_protocol.h
+++ b/src/kudu/common/wire_protocol.h
@@ -151,6 +151,9 @@ Status ExtraConfigPBToMap(const TableExtraConfigPB& pb,
 Status ExtraConfigPBFromPBMap(const google::protobuf::Map<std::string, std::string>&
configs,
                               TableExtraConfigPB* pb);
 
+// Parse int32_t type value from 'value', and store in 'result' when succeed.
+Status ParseInt32Config(const std::string& name, const std::string& value, int32_t*
result);
+
 // Convert a extra configuration properties protobuf to protobuf::map.
 Status ExtraConfigPBToPBMap(const TableExtraConfigPB& pb,
                             google::protobuf::Map<std::string, std::string>* configs);
diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc
index 79b7396..5ce0e12 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -64,6 +64,7 @@
 #include "kudu/tserver/mini_tablet_server.h"
 #include "kudu/tserver/tablet_server.h"
 #include "kudu/tserver/ts_tablet_manager.h"
+#include "kudu/util/maintenance_manager.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/random.h"
 #include "kudu/util/scoped_cleanup.h"
@@ -102,6 +103,7 @@ using kudu::cluster::InternalMiniClusterOptions;
 using kudu::master::AlterTableRequestPB;
 using kudu::master::AlterTableResponsePB;
 using kudu::tablet::TabletReplica;
+using kudu::tablet::Tablet;
 using std::atomic;
 using std::map;
 using std::pair;
@@ -275,6 +277,15 @@ class AlterTableTest : public KuduTest {
                      vector<unique_ptr<KuduPartialRow>> split_rows,
                      vector<pair<unique_ptr<KuduPartialRow>, unique_ptr<KuduPartialRow>>>
bounds);
 
+  void CheckMaintenancePriority(int32_t expect_priority) {
+    for (auto op : tablet_replica_->maintenance_ops_) {
+      ASSERT_EQ(op->priority(), expect_priority);
+    }
+    for (auto op : tablet_replica_->tablet()->maintenance_ops_) {
+      ASSERT_EQ(op->priority(), expect_priority);
+    }
+  }
+
  protected:
   virtual int num_replicas() const { return 1; }
 
@@ -2124,4 +2135,23 @@ TEST_F(AlterTableTest, TestKUDU2132) {
   ASSERT_OK(table_alterer->Alter());
 }
 
+TEST_F(AlterTableTest, TestMaintenancePriorityAlter) {
+  // Default priority level.
+  NO_FATALS(CheckMaintenancePriority(0));
+
+  // Set to some priority level.
+  unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
+  ASSERT_OK(table_alterer->AlterExtraConfig({{"kudu.table.maintenance_priority", "3"}})->Alter());
+  NO_FATALS(CheckMaintenancePriority(3));
+
+  // Set to another priority level.
+  ASSERT_OK(table_alterer->AlterExtraConfig({{"kudu.table.maintenance_priority", "-1"}})->Alter());
+  NO_FATALS(CheckMaintenancePriority(-1));
+
+  // Reset to default priority level.
+  ASSERT_OK(table_alterer->AlterExtraConfig({{"kudu.table.maintenance_priority", "0"}})->Alter());
+  NO_FATALS(CheckMaintenancePriority(0));
+}
+
+
 } // namespace kudu
diff --git a/src/kudu/tablet/tablet.h b/src/kudu/tablet/tablet.h
index 31ef480..7110a5b 100644
--- a/src/kudu/tablet/tablet.h
+++ b/src/kudu/tablet/tablet.h
@@ -53,7 +53,7 @@
 #include "kudu/util/status.h"
 
 namespace kudu {
-
+class AlterTableTest;
 class ConstContiguousRow;
 class EncodedKey;
 class KeyRange;
@@ -420,7 +420,6 @@ class Tablet {
   // This method is thread-safe.
   void CancelMaintenanceOps();
 
-  const std::string& table_id() const { return metadata_->table_id(); }
   const std::string& tablet_id() const { return metadata_->tablet_id(); }
 
   // Return the metrics for this tablet.
@@ -460,6 +459,7 @@ class Tablet {
                      std::vector<KeyRange>* ranges);
 
  private:
+  friend class kudu::AlterTableTest;
   friend class Iterator;
   friend class TabletReplicaTest;
   FRIEND_TEST(TestTablet, TestGetReplaySizeForIndex);
diff --git a/src/kudu/tablet/tablet_mm_ops.cc b/src/kudu/tablet/tablet_mm_ops.cc
index 5948061..849d74a 100644
--- a/src/kudu/tablet/tablet_mm_ops.cc
+++ b/src/kudu/tablet/tablet_mm_ops.cc
@@ -21,13 +21,16 @@
 #include <ostream>
 #include <utility>
 
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 
+#include "kudu/common/common.pb.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tablet/rowset.h"
 #include "kudu/tablet/tablet.h"
+#include "kudu/tablet/tablet_metadata.h"
 #include "kudu/tablet/tablet_metrics.h"
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/logging.h"
@@ -88,8 +91,13 @@ string TabletOpBase::LogPrefix() const {
   return tablet_->LogPrefix();
 }
 
-const std::string& TabletOpBase::table_id() const {
-  return tablet_->table_id();
+int32_t TabletOpBase::priority() const {
+  int32_t priority = 0;
+  const auto& extra_config = tablet_->metadata()->extra_config();
+  if (extra_config && extra_config->has_maintenance_priority()) {
+    priority = extra_config->maintenance_priority();
+  }
+  return priority;
 }
 
 ////////////////////////////////////////////////////////////
diff --git a/src/kudu/tablet/tablet_mm_ops.h b/src/kudu/tablet/tablet_mm_ops.h
index 6180652..e85dcf0 100644
--- a/src/kudu/tablet/tablet_mm_ops.h
+++ b/src/kudu/tablet/tablet_mm_ops.h
@@ -42,7 +42,7 @@ class TabletOpBase : public MaintenanceOp {
   std::string LogPrefix() const;
 
  protected:
-  const std::string& table_id() const override;
+  int32_t priority() const override;
 
   Tablet* const tablet_;
 };
diff --git a/src/kudu/tablet/tablet_replica.h b/src/kudu/tablet/tablet_replica.h
index 1bcffa7..044f6a8 100644
--- a/src/kudu/tablet/tablet_replica.h
+++ b/src/kudu/tablet/tablet_replica.h
@@ -47,6 +47,7 @@
 #include "kudu/util/status.h"
 
 namespace kudu {
+class AlterTableTest;
 class DnsResolver;
 class MaintenanceManager;
 class MaintenanceOp;
@@ -256,7 +257,6 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
     return log_anchor_registry_;
   }
 
-  const std::string& table_id() const { return meta_->table_id(); }
   const std::string& tablet_id() const { return meta_->tablet_id(); }
 
   // Convenience method to return the permanent_uuid of this peer.
@@ -301,6 +301,7 @@ class TabletReplica : public RefCountedThreadSafe<TabletReplica>,
   size_t OnDiskSize() const;
 
  private:
+  friend class kudu::AlterTableTest;
   friend class RefCountedThreadSafe<TabletReplica>;
   friend class TabletReplicaTest;
   FRIEND_TEST(TabletReplicaTest, TestMRSAnchorPreventsLogGC);
diff --git a/src/kudu/tablet/tablet_replica_mm_ops.cc b/src/kudu/tablet/tablet_replica_mm_ops.cc
index 08b7ac6..10713bb 100644
--- a/src/kudu/tablet/tablet_replica_mm_ops.cc
+++ b/src/kudu/tablet/tablet_replica_mm_ops.cc
@@ -23,12 +23,15 @@
 #include <string>
 #include <utility>
 
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 
+#include "kudu/common/common.pb.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/substitute.h"
+#include "kudu/tablet/tablet_metadata.h"
 #include "kudu/tablet/tablet_metrics.h"
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/logging.h"
@@ -133,8 +136,13 @@ TabletReplicaOpBase::TabletReplicaOpBase(std::string name,
       tablet_replica_(tablet_replica) {
 }
 
-const std::string& TabletReplicaOpBase::table_id() const {
-  return tablet_replica_->table_id();
+int32_t TabletReplicaOpBase::priority() const {
+  int32_t priority = 0;
+  const auto& extra_config = tablet_replica_->tablet_metadata()->extra_config();
+  if (extra_config && extra_config->has_maintenance_priority()) {
+    priority = extra_config->maintenance_priority();
+  }
+  return priority;
 }
 
 //
diff --git a/src/kudu/tablet/tablet_replica_mm_ops.h b/src/kudu/tablet/tablet_replica_mm_ops.h
index 62b4cb9..fd1dd5b 100644
--- a/src/kudu/tablet/tablet_replica_mm_ops.h
+++ b/src/kudu/tablet/tablet_replica_mm_ops.h
@@ -51,7 +51,7 @@ class TabletReplicaOpBase : public MaintenanceOp {
   explicit TabletReplicaOpBase(std::string name, IOUsage io_usage, TabletReplica* tablet_replica);
 
  protected:
-  const std::string& table_id() const override;
+  int32_t priority() const override;
 
   TabletReplica *const tablet_replica_;
 };
diff --git a/src/kudu/util/maintenance_manager-test.cc b/src/kudu/util/maintenance_manager-test.cc
index 8619453..d75f9a1 100644
--- a/src/kudu/util/maintenance_manager-test.cc
+++ b/src/kudu/util/maintenance_manager-test.cc
@@ -30,7 +30,6 @@
 #include <utility>
 
 #include <boost/bind.hpp> // IWYU pragma: keep
-#include <gflags/gflags.h>
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #include <gtest/gtest.h>
@@ -61,7 +60,6 @@ METRIC_DEFINE_histogram(test, maintenance_op_duration,
                         kudu::MetricUnit::kSeconds, "", 60000000LU, 2);
 
 DECLARE_int64(log_target_replay_size_mb);
-DECLARE_string(maintenance_manager_table_priorities);
 DECLARE_double(maintenance_op_multiplier);
 DECLARE_int32(max_priority_range);
 
@@ -112,7 +110,7 @@ class TestMaintenanceOp : public MaintenanceOp {
  public:
   TestMaintenanceOp(const std::string& name,
                     IOUsage io_usage,
-                    std::string table_id = "fake.table_id")
+                    int32_t priority = 0)
     : MaintenanceOp(name, io_usage),
       ram_anchored_(500),
       logs_retained_bytes_(0),
@@ -123,7 +121,7 @@ class TestMaintenanceOp : public MaintenanceOp {
       remaining_runs_(1),
       prepared_runs_(0),
       sleep_time_(MonoDelta::FromSeconds(0)),
-      table_id_(std::move(table_id)) {
+      priority_(priority) {
   }
 
   ~TestMaintenanceOp() override = default;
@@ -194,8 +192,8 @@ class TestMaintenanceOp : public MaintenanceOp {
     return maintenance_ops_running_;
   }
 
-  const std::string& table_id() const override {
-    return table_id_;
+  int32_t priority() const override {
+    return priority_;
   }
 
  private:
@@ -217,7 +215,9 @@ class TestMaintenanceOp : public MaintenanceOp {
 
   // The amount of time each op invocation will sleep.
   MonoDelta sleep_time_;
-  std::string table_id_;
+
+  // Maintenance priority.
+  int32_t priority_;
 };
 
 // Create an op and wait for it to start running.  Unregister it while it is
@@ -397,28 +397,20 @@ TEST_F(MaintenanceManagerTest, TestOpFactors) {
   StopManager();
 
   ASSERT_GE(FLAGS_max_priority_range, 1);
-  ASSERT_NE("", gflags::SetCommandLineOption(
-      "maintenance_manager_table_priorities",
-      Substitute("table_id_1:$0;table_id_2:$1;table_id_3:$2;table_id_4:$3;table_id_5:$4",
-                 -FLAGS_max_priority_range - 1, -1, 0, 1, FLAGS_max_priority_range + 1).c_str()));
-  TestMaintenanceOp op1("op1", MaintenanceOp::HIGH_IO_USAGE, "table_id_1");
-  TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, "table_id_2");
-  TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, "table_id_3");
-  TestMaintenanceOp op4("op4", MaintenanceOp::HIGH_IO_USAGE, "table_id_4");
-  TestMaintenanceOp op5("op5", MaintenanceOp::HIGH_IO_USAGE, "table_id_5");
-  TestMaintenanceOp op6("op6", MaintenanceOp::HIGH_IO_USAGE, "table_id_6");
-
-  manager_->UpdateTablePriorities();
+  TestMaintenanceOp op1("op1", MaintenanceOp::HIGH_IO_USAGE, -FLAGS_max_priority_range -
1);
+  TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, -1);
+  TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, 0);
+  TestMaintenanceOp op4("op4", MaintenanceOp::HIGH_IO_USAGE, 1);
+  TestMaintenanceOp op5("op5", MaintenanceOp::HIGH_IO_USAGE, FLAGS_max_priority_range + 1);
 
   ASSERT_DOUBLE_EQ(pow(FLAGS_maintenance_op_multiplier, -FLAGS_max_priority_range),
-                   manager_->PerfImprovement(1, op1.table_id()));
+                   manager_->PerfImprovement(1, op1.priority()));
   ASSERT_DOUBLE_EQ(pow(FLAGS_maintenance_op_multiplier, -1),
-                   manager_->PerfImprovement(1, op2.table_id()));
-  ASSERT_DOUBLE_EQ(1, manager_->PerfImprovement(1, op3.table_id()));
-  ASSERT_DOUBLE_EQ(FLAGS_maintenance_op_multiplier, manager_->PerfImprovement(1, op4.table_id()));
+                   manager_->PerfImprovement(1, op2.priority()));
+  ASSERT_DOUBLE_EQ(1, manager_->PerfImprovement(1, op3.priority()));
+  ASSERT_DOUBLE_EQ(FLAGS_maintenance_op_multiplier, manager_->PerfImprovement(1, op4.priority()));
   ASSERT_DOUBLE_EQ(pow(FLAGS_maintenance_op_multiplier, FLAGS_max_priority_range),
-                   manager_->PerfImprovement(1, op5.table_id()));
-  ASSERT_DOUBLE_EQ(1, manager_->PerfImprovement(1, op6.table_id()));
+                   manager_->PerfImprovement(1, op5.priority()));
 }
 
 // Test priority OP launching.
@@ -426,37 +418,32 @@ TEST_F(MaintenanceManagerTest, TestPriorityOpLaunch) {
   StopManager();
   StartManager(1);
 
-  ASSERT_NE("", gflags::SetCommandLineOption(
-      "maintenance_manager_table_priorities",
-      Substitute("table_id_1:$0;table_id_2:$1;table_id_3:$2;table_id_4:$3;table_id_5:$4",
-                 -FLAGS_max_priority_range - 1, -1, 0, 1, FLAGS_max_priority_range + 1).c_str()));
-
-  TestMaintenanceOp op1("op1", MaintenanceOp::HIGH_IO_USAGE, "table_id_1");
+  TestMaintenanceOp op1("op1", MaintenanceOp::HIGH_IO_USAGE, -FLAGS_max_priority_range -
1);
   op1.set_perf_improvement(10);
   op1.set_remaining_runs(1);
   op1.set_sleep_time(MonoDelta::FromMilliseconds(1));
 
-  TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, "table_id_2");
+  TestMaintenanceOp op2("op2", MaintenanceOp::HIGH_IO_USAGE, -1);
   op2.set_perf_improvement(10);
   op2.set_remaining_runs(1);
   op2.set_sleep_time(MonoDelta::FromMilliseconds(1));
 
-  TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, "table_id_3");
+  TestMaintenanceOp op3("op3", MaintenanceOp::HIGH_IO_USAGE, 0);
   op3.set_perf_improvement(10);
   op3.set_remaining_runs(1);
   op3.set_sleep_time(MonoDelta::FromMilliseconds(1));
 
-  TestMaintenanceOp op4("op4", MaintenanceOp::HIGH_IO_USAGE, "table_id_4");
+  TestMaintenanceOp op4("op4", MaintenanceOp::HIGH_IO_USAGE, 1);
   op4.set_perf_improvement(10);
   op4.set_remaining_runs(1);
   op4.set_sleep_time(MonoDelta::FromMilliseconds(1));
 
-  TestMaintenanceOp op5("op5", MaintenanceOp::HIGH_IO_USAGE, "table_id_5");
+  TestMaintenanceOp op5("op5", MaintenanceOp::HIGH_IO_USAGE, FLAGS_max_priority_range + 1);
   op5.set_perf_improvement(10);
   op5.set_remaining_runs(1);
   op5.set_sleep_time(MonoDelta::FromMilliseconds(1));
 
-  TestMaintenanceOp op6("op6", MaintenanceOp::HIGH_IO_USAGE, "table_id_6");
+  TestMaintenanceOp op6("op6", MaintenanceOp::HIGH_IO_USAGE, 0);
   op6.set_perf_improvement(12);
   op6.set_remaining_runs(1);
   op6.set_sleep_time(MonoDelta::FromMilliseconds(1));
diff --git a/src/kudu/util/maintenance_manager.cc b/src/kudu/util/maintenance_manager.cc
index f783717..b8b3a29 100644
--- a/src/kudu/util/maintenance_manager.cc
+++ b/src/kudu/util/maintenance_manager.cc
@@ -35,7 +35,6 @@
 #include "kudu/gutil/dynamic_annotations.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stringprintf.h"
-#include "kudu/gutil/strings/numbers.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/debug/trace_event.h"
@@ -97,14 +96,6 @@ DEFINE_double(data_gc_prioritization_prob, 0.5,
              "such as delta compaction.");
 TAG_FLAG(data_gc_prioritization_prob, experimental);
 
-DEFINE_string(maintenance_manager_table_priorities, "",
-              "Priorities of tables, semicolon-separated list of table-priority pairs, and
each "
-              "table-priority pair is combined by table id, colon and priority level. Priority
"
-              "level is ranged in [-FLAGS_max_priority_range, FLAGS_max_priority_range]");
-TAG_FLAG(maintenance_manager_table_priorities, advanced);
-TAG_FLAG(maintenance_manager_table_priorities, experimental);
-TAG_FLAG(maintenance_manager_table_priorities, runtime);
-
 DEFINE_double(maintenance_op_multiplier, 1.1,
               "Multiplier applied on different priority levels, table maintenance OPs on
level N "
               "has multiplier of FLAGS_maintenance_op_multiplier^N, the last score will be
"
@@ -302,9 +293,6 @@ void MaintenanceManager::RunSchedulerThread() {
       return;
     }
 
-    // TODO(yingchun): move it to SetFlag, callback once as a gflags setter handler.
-    UpdateTablePriorities();
-
     // If we found no work to do, then we should sleep before trying again to schedule.
     // Otherwise, we can go right into trying to find the next op.
     prev_iter_found_no_work = !FindAndLaunchOp(&guard);
@@ -432,7 +420,7 @@ pair<MaintenanceOp*, string> MaintenanceManager::FindBestOp() {
                         op->name(), data_retained_bytes);
     }
 
-    const auto perf_improvement = PerfImprovement(stats.perf_improvement(), op->table_id());
+    const auto perf_improvement = PerfImprovement(stats.perf_improvement(), op->priority());
     if ((!best_perf_improvement_op) ||
         (perf_improvement > best_perf_improvement)) {
       best_perf_improvement_op = op;
@@ -491,13 +479,13 @@ pair<MaintenanceOp*, string> MaintenanceManager::FindBestOp()
{
   return {nullptr, "no ops with positive improvement"};
 }
 
-double MaintenanceManager::PerfImprovement(double perf_improvement,
-                                           const string& table_id) const {
-  int32_t priority = 0;
-  if (!FindCopy(table_priorities_, table_id, &priority)) {
+double MaintenanceManager::PerfImprovement(double perf_improvement, int32_t priority) const
{
+  if (priority == 0) {
     return perf_improvement;
   }
 
+  priority = std::max(priority, -FLAGS_max_priority_range);
+  priority = std::min(priority, FLAGS_max_priority_range);
   return perf_improvement * std::pow(FLAGS_maintenance_op_multiplier, priority);
 }
 
@@ -600,28 +588,6 @@ bool MaintenanceManager::CouldNotLaunchNewOp(bool prev_iter_found_no_work)
{
   return (!HasFreeThreads() || prev_iter_found_no_work || disabled_for_tests()) &&
!shutdown_;
 }
 
-void MaintenanceManager::UpdateTablePriorities() {
-  string table_priorities_str;
-  CHECK(google::GetCommandLineOption("maintenance_manager_table_priorities",
-                                     &table_priorities_str));
-
-  TablePriorities table_priorities;
-  int32_t value;
-  for (auto table_priority_str : Split(table_priorities_str, ";", strings::SkipEmpty()))
{
-    vector<string> table_priority = Split(table_priority_str, ":", strings::SkipEmpty());
-    if (safe_strto32_base(table_priority[1].c_str(), &value, 10)) {
-      value = std::max(value, -FLAGS_max_priority_range);
-      value = std::min(value, FLAGS_max_priority_range);
-      table_priorities[table_priority[0]] = value;
-    } else {
-      LOG(WARNING) << "Some error occured when parse flag maintenance_manager_table_priorities:
"
-          << table_priorities_str;
-      return;
-    }
-  }
-  table_priorities_.swap(table_priorities);
-}
-
 void MaintenanceManager::IncreaseOpCount(MaintenanceOp *op) {
   op->running_++;
   running_ops_++;
diff --git a/src/kudu/util/maintenance_manager.h b/src/kudu/util/maintenance_manager.h
index a4e6b96..95a7987 100644
--- a/src/kudu/util/maintenance_manager.h
+++ b/src/kudu/util/maintenance_manager.h
@@ -41,7 +41,6 @@
 #include "kudu/util/status.h"
 
 namespace kudu {
-
 template<class T>
 class AtomicGauge;
 class Histogram;
@@ -230,7 +229,9 @@ class MaintenanceOp {
   }
 
  protected:
-  virtual const std::string& table_id() const = 0;
+  friend class AlterTableTest;
+
+  virtual int32_t priority() const = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MaintenanceOp);
@@ -309,7 +310,6 @@ class MaintenanceManager : public std::enable_shared_from_this<MaintenanceManage
 
   typedef std::map<MaintenanceOp*, MaintenanceOpStats,
           MaintenanceOpComparator> OpMapTy;
-  typedef std::unordered_map<std::string, int32_t> TablePriorities;
 
   // Return true if tests have currently disabled the maintenance
   // manager by way of changing the gflags at runtime.
@@ -325,8 +325,7 @@ class MaintenanceManager : public std::enable_shared_from_this<MaintenanceManage
   // suitable for logging.
   std::pair<MaintenanceOp*, std::string> FindBestOp();
 
-  double PerfImprovement(double perf_improvement,
-                         const std::string& table_id) const;
+  double PerfImprovement(double perf_improvement, int32_t priority) const;
 
   void LaunchOp(MaintenanceOp* op);
 
@@ -336,13 +335,10 @@ class MaintenanceManager : public std::enable_shared_from_this<MaintenanceManage
 
   bool CouldNotLaunchNewOp(bool prev_iter_found_no_work);
 
-  void UpdateTablePriorities();
-
   void IncreaseOpCount(MaintenanceOp *op);
   void DecreaseOpCount(MaintenanceOp *op);
 
   const std::string server_uuid_;
-  TablePriorities table_priorities_;
   const int32_t num_threads_;
   OpMapTy ops_; // Registered operations.
   Mutex lock_;


Mime
View raw message