kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject kudu git commit: KUDU-1658: Reject CREATE TABLE ops with even replication factor
Date Wed, 30 Nov 2016 18:42:55 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 2b5ad4a9b -> c553a9a59


KUDU-1658: Reject CREATE TABLE ops with even replication factor

Reject table creation with even replication factor,
and add an allow_unsafe_replication_factor
master flag to make it possible for advanced user.

Change-Id: I1674cc59cdfc2955d42fc5e4d8c0d962d9cc8e8e
Reviewed-on: http://gerrit.cloudera.org:8080/4945
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/c553a9a5
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c553a9a5
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c553a9a5

Branch: refs/heads/master
Commit: c553a9a59ce664c38041fbb3809ab6402cdbc89b
Parents: 2b5ad4a
Author: hahao <hao.hao@cloudera.com>
Authored: Tue Nov 8 18:25:40 2016 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Nov 30 18:38:54 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/client-test.cc                  | 15 ++++++++++++
 src/kudu/integration-tests/delete_table-test.cc |  8 +++++--
 .../integration-tests/raft_consensus-itest.cc   | 24 ++++++++++++--------
 src/kudu/integration-tests/tablet_copy-itest.cc |  8 ++++---
 .../ts_tablet_manager-itest.cc                  |  4 ++++
 src/kudu/master/catalog_manager.cc              | 12 ++++++++++
 src/kudu/master/master.proto                    |  3 +++
 src/kudu/tools/kudu-admin-test.cc               |  3 ++-
 8 files changed, 62 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 58c48c6..c52cd76 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -72,6 +72,7 @@
 DECLARE_bool(enable_data_block_fsync);
 DECLARE_bool(fail_dns_resolution);
 DECLARE_bool(log_inject_latency);
+DECLARE_bool(allow_unsafe_replication_factor);
 DECLARE_int32(heartbeat_interval_ms);
 DECLARE_int32(log_inject_latency_ms_mean);
 DECLARE_int32(log_inject_latency_ms_stddev);
@@ -1144,6 +1145,9 @@ TEST_F(ClientTest, TestScanFaultTolerance) {
   const string kScanTable = "TestScanFaultTolerance";
   shared_ptr<KuduTable> table;
 
+  // Allow creating table with even replication factor.
+  FLAGS_allow_unsafe_replication_factor = true;
+
   // We use only two replicas in this test so that every write is fully replicated to both
   // servers (the Raft majority is 2/2). This reduces potential flakiness if the scanner
tries
   // to read from a replica that is lagging for some reason. This won't be necessary once
@@ -4223,5 +4227,16 @@ TEST_F(ClientTest, TestGetTablet) {
   }
 }
 
+// Test create table with even replicas factor should fail.
+TEST_F(ClientTest, TestTableWithEvenReplicas) {
+  gscoped_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  Status s = table_creator->table_name("table_with_even_replicas")
+                          .schema(&schema_)
+                          .num_replicas(2)
+                          .set_range_partition_columns({ "key" })
+                          .Create();
+  ASSERT_TRUE(s.IsInvalidArgument());
+  ASSERT_STR_CONTAINS(s.ToString(), "Illegal replication factor");
+}
 } // namespace client
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/integration-tests/delete_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/delete_table-test.cc b/src/kudu/integration-tests/delete_table-test.cc
index 0665d0f..08b48c3 100644
--- a/src/kudu/integration-tests/delete_table-test.cc
+++ b/src/kudu/integration-tests/delete_table-test.cc
@@ -397,7 +397,10 @@ TEST_F(DeleteTableTest, TestAutoTombstoneAfterCrashDuringTabletCopy)
{
     "--flush_threshold_mb=0",
     "--maintenance_manager_polling_interval_ms=100"
   };
-  NO_FATALS(StartCluster(ts_flags));
+  vector<string> master_flags = {
+    "--allow_unsafe_replication_factor=true"
+  };
+  NO_FATALS(StartCluster(ts_flags, master_flags));
   const MonoDelta timeout = MonoDelta::FromSeconds(10);
   const int kTsIndex = 0; // We'll test with the first TS.
 
@@ -553,7 +556,8 @@ TEST_F(DeleteTableTest, TestAutoTombstoneAfterTabletCopyRemoteFails) {
       "--log_segment_size_mb=1"                   // Faster log rolls.
   };
   vector<string> master_flags = {
-      "--catalog_manager_wait_for_new_tablets_to_elect_leader=false"
+      "--catalog_manager_wait_for_new_tablets_to_elect_leader=false",
+      "--allow_unsafe_replication_factor=true"
   };
   NO_FATALS(StartCluster(ts_flags, master_flags));
   const MonoDelta kTimeout = MonoDelta::FromSeconds(20);

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/integration-tests/raft_consensus-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 41dedb0..6582626 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -1982,7 +1982,8 @@ TEST_F(RaftConsensusITest, TestMasterNotifiedOnConfigChange) {
   FLAGS_num_tablet_servers = 3;
   FLAGS_num_replicas = 2;
   vector<string> ts_flags;
-  vector<string> master_flags = { "--master_add_server_when_underreplicated=false"
};
+  vector<string> master_flags = { "--master_add_server_when_underreplicated=false",
+                                  "--allow_unsafe_replication_factor=true"};
   NO_FATALS(BuildAndStart(ts_flags, master_flags));
 
   LOG(INFO) << "Finding tablet leader and waiting for things to start...";
@@ -2133,14 +2134,19 @@ TEST_F(RaftConsensusITest, TestEarlyCommitDespiteMemoryPressure) {
 TEST_F(RaftConsensusITest, TestAutoCreateReplica) {
   FLAGS_num_tablet_servers = 3;
   FLAGS_num_replicas = 2;
-  vector<string> ts_flags, master_flags;
-  ts_flags.push_back("--enable_leader_failure_detection=false");
-  ts_flags.push_back("--log_cache_size_limit_mb=1");
-  ts_flags.push_back("--log_segment_size_mb=1");
-  ts_flags.push_back("--log_async_preallocate_segments=false");
-  ts_flags.push_back("--flush_threshold_mb=1");
-  ts_flags.push_back("--maintenance_manager_polling_interval_ms=300");
-  master_flags.push_back("--catalog_manager_wait_for_new_tablets_to_elect_leader=false");
+
+  vector<string> ts_flags = {
+      "--enable_leader_failure_detection=false",
+      "--log_cache_size_limit_mb=1",
+      "--log_segment_size_mb=1",
+      "--log_async_preallocate_segments=false",
+      "--flush_threshold_mb=1",
+      "--maintenance_manager_polling_interval_ms=300",
+  };
+  vector<string> master_flags = {
+      "--catalog_manager_wait_for_new_tablets_to_elect_leader=false",
+      "--allow_unsafe_replication_factor=true"
+  };
   BuildAndStart(ts_flags, master_flags);
 
   // 50K is enough to cause flushes & log rolls.

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/integration-tests/tablet_copy-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_copy-itest.cc b/src/kudu/integration-tests/tablet_copy-itest.cc
index b201d0a..071f18c 100644
--- a/src/kudu/integration-tests/tablet_copy-itest.cc
+++ b/src/kudu/integration-tests/tablet_copy-itest.cc
@@ -409,9 +409,11 @@ TEST_F(TabletCopyITest, TestDeleteTabletDuringTabletCopy) {
 // as the tablet copy source. When a tablet is tombstoned, its last-logged
 // opid is stored in a field its on-disk superblock.
 TEST_F(TabletCopyITest, TestTabletCopyFollowerWithHigherTerm) {
-  vector<string> ts_flags, master_flags;
-  ts_flags.push_back("--enable_leader_failure_detection=false");
-  master_flags.push_back("--catalog_manager_wait_for_new_tablets_to_elect_leader=false");
+  vector<string> ts_flags = { "--enable_leader_failure_detection=false" };
+  vector<string> master_flags = {
+      "--catalog_manager_wait_for_new_tablets_to_elect_leader=false",
+      "--allow_unsafe_replication_factor=true"
+  };
   const int kNumTabletServers = 2;
   NO_FATALS(StartCluster(ts_flags, master_flags, kNumTabletServers));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/integration-tests/ts_tablet_manager-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/ts_tablet_manager-itest.cc b/src/kudu/integration-tests/ts_tablet_manager-itest.cc
index 3ada5ce..a963eb9 100644
--- a/src/kudu/integration-tests/ts_tablet_manager-itest.cc
+++ b/src/kudu/integration-tests/ts_tablet_manager-itest.cc
@@ -44,6 +44,7 @@
 
 DECLARE_bool(enable_leader_failure_detection);
 DECLARE_bool(catalog_manager_wait_for_new_tablets_to_elect_leader);
+DECLARE_bool(allow_unsafe_replication_factor);
 DEFINE_int32(num_election_test_loops, 3,
              "Number of random EmulateElection() loops to execute in "
              "TestReportNewLeaderOnLeaderChange");
@@ -107,6 +108,9 @@ TEST_F(TsTabletManagerITest, TestReportNewLeaderOnLeaderChange) {
   FLAGS_enable_leader_failure_detection = false;
   FLAGS_catalog_manager_wait_for_new_tablets_to_elect_leader = false;
 
+  // Allow creating table with even replication factor.
+  FLAGS_allow_unsafe_replication_factor = true;
+
   // Run a few more iters in slow-test mode.
   OverrideFlagForSlowTests("num_election_test_loops", "10");
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 472316c..67be524 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -116,6 +116,10 @@ DEFINE_int32(default_num_replicas, 3,
              "Default number of replicas for tables that do not have the num_replicas set.");
 TAG_FLAG(default_num_replicas, advanced);
 
+DEFINE_bool(allow_unsafe_replication_factor, false,
+            "Allow creating tables with even replication factor.");
+TAG_FLAG(allow_unsafe_replication_factor, unsafe);
+
 DEFINE_int32(catalog_manager_bg_task_wait_ms, 1000,
              "Amount of time the catalog manager background task thread waits "
              "between runs");
@@ -926,6 +930,14 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
     req.set_num_replicas(FLAGS_default_num_replicas);
   }
 
+  // Reject create table with even replication factors, unless master flag
+  // allow_unsafe_replication_factor is on.
+  if (req.num_replicas() % 2 == 0 && !FLAGS_allow_unsafe_replication_factor) {
+    s = Status::InvalidArgument(Substitute("Illegal replication factor $0 (replication "
+                                           "factor must be odd)", req.num_replicas()));
+    return SetError(MasterErrorPB::EVEN_REPLICATION_FACTOR, s);
+  }
+
   // Verify that the total number of tablets is reasonable, relative to the number
   // of live tablet servers.
   TSDescriptorVector ts_descs;

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 7b013c4..17a263d 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -63,6 +63,9 @@ message MasterErrorPB {
 
     // The request or response involved a tablet which is not yet running.
     TABLET_NOT_RUNNING = 9;
+
+    // The number of replicas requested is even.
+    EVEN_REPLICATION_FACTOR = 10;
   }
 
   // The error code.

http://git-wip-us.apache.org/repos/asf/kudu/blob/c553a9a5/src/kudu/tools/kudu-admin-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-admin-test.cc b/src/kudu/tools/kudu-admin-test.cc
index 0272550..1a34718 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -57,7 +57,8 @@ TEST_F(AdminCliTest, TestChangeConfig) {
   FLAGS_num_tablet_servers = 3;
   FLAGS_num_replicas = 2;
   BuildAndStart({ "--enable_leader_failure_detection=false" },
-                { "--catalog_manager_wait_for_new_tablets_to_elect_leader=false" });
+                { "--catalog_manager_wait_for_new_tablets_to_elect_leader=false",
+                  "--allow_unsafe_replication_factor=true"});
 
   vector<TServerDetails*> tservers;
   AppendValuesFromMap(tablet_servers_, &tservers);


Mime
View raw message