Merge pull request #2599 from georgelorchpercona/ps-5.7-4397-4946

Ps 5.7 4397 4946

mysql-test/suite/rocksdb.sys_vars/r/rocksdb_no_create_column_family_basic.result

@@ -0,0 +1,7 @@
+SET @start_global_value = @@global.ROCKSDB_NO_CREATE_COLUMN_FAMILY;
+SELECT @start_global_value;
+@start_global_value
+0
+"Trying to set variable @@global.ROCKSDB_NO_CREATE_COLUMN_FAMILY to 444. It should fail because it is readonly."
+SET @@global.ROCKSDB_NO_CREATE_COLUMN_FAMILY   = 444;
+ERROR HY000: Variable 'rocksdb_no_create_column_family' is a read only variable

mysql-test/suite/rocksdb.sys_vars/t/rocksdb_no_create_column_family_basic.test

@@ -0,0 +1,6 @@
+--source include/have_rocksdb.inc
+
+--let $sys_var=ROCKSDB_NO_CREATE_COLUMN_FAMILY
+--let $read_only=1
+--let $session=0
+--source ../include/rocksdb_sys_var.inc

mysql-test/suite/rocksdb/r/rocksdb.result

@@ -967,6 +967,7 @@ rocksdb_merge_combine_read_size	1073741824
 rocksdb_merge_tmp_file_removal_delay_ms	0
 rocksdb_new_table_reader_for_compaction_inputs	OFF
 rocksdb_no_block_cache	OFF
+rocksdb_no_create_column_family	OFF
 rocksdb_override_cf_options	
 rocksdb_paranoid_checks	ON
 rocksdb_pause_background_work	ON

mysql-test/suite/rocksdb/r/rocksdb_cf_options.result

@@ -59,6 +59,15 @@ z	WRITE_BUFFER_SIZE	12582912
 __system__	MAX_BYTES_FOR_LEVEL_MULTIPLIER	10.000000
 __system__	TARGET_FILE_SIZE_BASE	1048576
 __system__	WRITE_BUFFER_SIZE	12582912
+# restart:--rocksdb_no_create_column_family=1 --rocksdb_override_cf_options=cf3={write_buffer_size=8m;target_file_size_base=2m};cf4={write_buffer_size=16m;max_bytes_for_level_multiplier=8};
+include/assert.inc ["Expected cf3 and cf4 to not exist"]
+create table t4 (a int,
+primary key (a) comment 'cf3') engine=rocksdb;
+include/assert.inc ["Expected cf3 to exist"]
+create table t5 (a int,
+primary key (a) comment 'nobodyknows') engine=rocksdb;
+ERROR HY000: Incorrect arguments to CREATE | ALTER - can not find column family for storing index data and creation is not allowed.
+include/assert.inc ["Expected cf4 to NOT exist"]
 # restart
 
-drop table t1,t2,t3;
+drop table t1,t2,t3,t4;

mysql-test/suite/rocksdb/t/rocksdb_cf_options.test

@@ -49,6 +49,30 @@ select cf_name, option_type, value
                         'MAX_BYTES_FOR_LEVEL_MULTIPLIER')
   order by cf_name, option_type;
 
+# restart with no_create_column_family and column family options for new cf which is not yet created
+
+--let $restart_parameters=restart:--rocksdb_no_create_column_family=1 --rocksdb_override_cf_options=cf3={write_buffer_size=8m;target_file_size_base=2m};cf4={write_buffer_size=16m;max_bytes_for_level_multiplier=8};
+--source include/restart_mysqld.inc
+
+--let $assert_text="Expected cf3 and cf4 to not exist"
+--let $assert_cond="[select count(distinct cf_name) from information_schema.rocksdb_cf_options where cf_name in (\'cf3\', \'cf4\')]" = 0
+--source include/assert.inc
+
+create table t4 (a int,
+                 primary key (a) comment 'cf3') engine=rocksdb;
+
+--let $assert_text="Expected cf3 to exist"
+--let $assert_cond="[select count(distinct cf_name) from information_schema.rocksdb_cf_options where cf_name in (\'cf3\')]" = 1
+--source include/assert.inc
+
+--error ER_WRONG_ARGUMENTS
+create table t5 (a int,
+                 primary key (a) comment 'nobodyknows') engine=rocksdb;
+
+--let $assert_text="Expected cf4 to NOT exist"
+--let $assert_cond="[select count(distinct cf_name) from information_schema.rocksdb_cf_options where cf_name in (\'cf4\')]" = 0
+--source include/assert.inc
+
 # syntax error in options (no equal sign)
 
 --exec echo "restart:--rocksdb_override_cf_options=cf1" > $_expect_file_name
@@ -73,5 +97,4 @@ select cf_name, option_type, value
 --source include/restart_mysqld.inc
 
 --echo
-drop table t1,t2,t3;
-
+drop table t1,t2,t3,t4;

storage/rocksdb/ha_rocksdb.cc

@@ -500,6 +500,7 @@ static uint64_t rocksdb_write_policy =
     rocksdb::TxnDBWritePolicy::WRITE_COMMITTED;
 static my_bool rocksdb_error_on_suboptimal_collation = FALSE;
 static uint32_t rocksdb_stats_recalc_rate = 0;
+static my_bool rocksdb_no_create_column_family = FALSE;
 
 std::atomic<uint64_t> rocksdb_row_lock_deadlocks(0);
 std::atomic<uint64_t> rocksdb_row_lock_wait_timeouts(0);
@@ -1214,7 +1215,8 @@ static MYSQL_SYSVAR_STR(override_cf_options, rocksdb_override_cf_options,
                         "");
 
 static MYSQL_SYSVAR_STR(update_cf_options, rocksdb_update_cf_options,
-                        PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_MEMALLOC,
+                        PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_MEMALLOC |
+                            PLUGIN_VAR_NOCMDOPT,
                         "Option updates per column family for RocksDB",
                         rocksdb_validate_update_cf_options,
                         rocksdb_set_update_cf_options, nullptr);
@@ -1518,6 +1520,12 @@ static MYSQL_SYSVAR_BOOL(error_on_suboptimal_collation,
                          "collation is used",
                          nullptr, nullptr, false);
 
+static MYSQL_SYSVAR_BOOL(
+    no_create_column_family, rocksdb_no_create_column_family,
+    PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_READONLY,
+    "Do not allow creation of new Column Families through index comments.",
+    nullptr, nullptr, false);
+
 static const int ROCKSDB_ASSUMED_KEY_VALUE_DISK_SIZE = 100;
 
 static struct st_mysql_sys_var *rocksdb_system_variables[] = {
@@ -1665,6 +1673,7 @@ static struct st_mysql_sys_var *rocksdb_system_variables[] = {
     MYSQL_SYSVAR(large_prefix),
     MYSQL_SYSVAR(allow_to_start_after_corruption),
     MYSQL_SYSVAR(error_on_suboptimal_collation),
+    MYSQL_SYSVAR(no_create_column_family),
     MYSQL_SYSVAR(stats_recalc_rate),
     nullptr};
 
@@ -6240,7 +6249,8 @@ int ha_rocksdb::create_cfs(
     // Here's how `get_or_create_cf` will use the input parameters:
     //
     // `cf_name` - will be used as a CF name.
-    cf_handle = cf_manager.get_or_create_cf(rdb, cf_name);
+    cf_handle = cf_manager.get_or_create_cf(rdb, cf_name,
+                                            !rocksdb_no_create_column_family);
 
     if (!cf_handle) {
       DBUG_RETURN(HA_EXIT_FAILURE);

storage/rocksdb/rdb_cf_manager.cc

@@ -67,14 +67,14 @@ void Rdb_cf_manager::cleanup() {
 
 /*
   @brief
-  Find column family by name. If it doesn't exist, create it
+  Find column family by name. If it doesn't exist, maybe create it
 
   @detail
     See Rdb_cf_manager::get_cf
 */
 rocksdb::ColumnFamilyHandle *
 Rdb_cf_manager::get_or_create_cf(rocksdb::DB *const rdb,
-                                 const std::string &cf_name_arg) {
+                                 const std::string &cf_name_arg, bool create) {
   DBUG_ASSERT(rdb != nullptr);
 
   rocksdb::ColumnFamilyHandle *cf_handle = nullptr;
@@ -97,23 +97,32 @@ Rdb_cf_manager::get_or_create_cf(rocksdb::DB *const rdb,
   } else {
     /* Create a Column Family. */
     rocksdb::ColumnFamilyOptions opts;
-    m_cf_options->get_cf_options(cf_name, &opts);
-
-    // NO_LINT_DEBUG
-    sql_print_information("RocksDB: creating a column family %s",
-                          cf_name.c_str());
-    sql_print_information("    write_buffer_size=%ld", opts.write_buffer_size);
-    sql_print_information("    target_file_size_base=%" PRIu64,
-                          opts.target_file_size_base);
-
-    const rocksdb::Status s =
-        rdb->CreateColumnFamily(opts, cf_name, &cf_handle);
-
-    if (s.ok()) {
-      m_cf_name_map[cf_handle->GetName()] = cf_handle;
-      m_cf_id_map[cf_handle->GetID()] = cf_handle;
+    bool cf_name_found = m_cf_options->get_cf_options(cf_name, &opts);
+
+    if (create || cf_name_found) {
+      // NO_LINT_DEBUG
+      sql_print_information("RocksDB: creating a column family %s",
+                            cf_name.c_str());
+      sql_print_information("    write_buffer_size=%ld",
+                            opts.write_buffer_size);
+      sql_print_information("    target_file_size_base=%" PRIu64,
+                            opts.target_file_size_base);
+
+      const rocksdb::Status s =
+          rdb->CreateColumnFamily(opts, cf_name, &cf_handle);
+
+      if (s.ok()) {
+        m_cf_name_map[cf_handle->GetName()] = cf_handle;
+        m_cf_id_map[cf_handle->GetID()] = cf_handle;
+      } else {
+        cf_handle = nullptr;
+      }
     } else {
-      cf_handle = nullptr;
+      RDB_MUTEX_UNLOCK_CHECK(m_mutex);
+      my_error(ER_WRONG_ARGUMENTS, MYF(0),
+               "CREATE | ALTER - can not find column family for storing index "
+               "data and creation is not allowed.");
+      return nullptr;
     }
   }
 

storage/rocksdb/rdb_cf_manager.h

@@ -73,9 +73,11 @@ class Rdb_cf_manager {
   /*
     Used by CREATE TABLE.
     - cf_name=nullptr means use default column family
+    - create=true means create cf if missing, otherwise return nullptr
   */
   rocksdb::ColumnFamilyHandle *get_or_create_cf(rocksdb::DB *const rdb,
-                                                const std::string &cf_name);
+                                                const std::string &cf_name,
+                                                bool create);
 
   /* Used by table open */
   rocksdb::ColumnFamilyHandle *get_cf(const std::string &cf_name) const;

storage/rocksdb/rdb_cf_options.cc

@@ -68,7 +68,7 @@ bool Rdb_cf_options::init(
   return true;
 }
 
-void Rdb_cf_options::get(const std::string &cf_name,
+bool Rdb_cf_options::get(const std::string &cf_name,
                          rocksdb::ColumnFamilyOptions *const opts) {
   DBUG_ASSERT(opts != nullptr);
 
@@ -80,7 +80,9 @@ void Rdb_cf_options::get(const std::string &cf_name,
 
   if (it != m_name_map.end()) {
     rocksdb::GetColumnFamilyOptionsFromString(*opts, it->second, opts);
+    return true;
   }
+  return false;
 }
 
 void Rdb_cf_options::update(const std::string &cf_name,
@@ -330,16 +332,17 @@ Rdb_cf_options::get_cf_merge_operator(const std::string &cf_name) {
              : nullptr;
 }
 
-void Rdb_cf_options::get_cf_options(const std::string &cf_name,
+bool Rdb_cf_options::get_cf_options(const std::string &cf_name,
                                     rocksdb::ColumnFamilyOptions *const opts) {
   DBUG_ASSERT(opts != nullptr);
 
   *opts = m_default_cf_opts;
-  get(cf_name, opts);
+  bool ret = get(cf_name, opts);
 
   // Set the comparator according to 'rev:'
   opts->comparator = get_cf_comparator(cf_name);
   opts->merge_operator = get_cf_merge_operator(cf_name);
+  return ret;
 }
 
 } // namespace myrocks

storage/rocksdb/rdb_cf_options.h

@@ -46,8 +46,9 @@ class Rdb_cf_options {
   Rdb_cf_options &operator=(const Rdb_cf_options &) = delete;
   Rdb_cf_options() = default;
 
-  void get(const std::string &cf_name,
-           rocksdb::ColumnFamilyOptions *const opts);
+  /* bool true return indicates cf_name was found */
+  MY_NODISCARD bool get(const std::string &cf_name,
+                        rocksdb::ColumnFamilyOptions *const opts);
 
   void update(const std::string &cf_name, const std::string &cf_options);
 
@@ -67,7 +68,8 @@ class Rdb_cf_options {
   std::shared_ptr<rocksdb::MergeOperator>
   get_cf_merge_operator(const std::string &cf_name);
 
-  void get_cf_options(const std::string &cf_name,
+  /* bool true return indicates cf_name was found, otherwise default */
+  bool get_cf_options(const std::string &cf_name,
                       rocksdb::ColumnFamilyOptions *const opts);
 
   static bool parse_cf_options(const std::string &cf_options,

storage/rocksdb/rdb_datadic.cc

@@ -4895,7 +4895,8 @@ bool Rdb_dict_manager::init(rocksdb::TransactionDB *const rdb_dict,
 
   m_db = rdb_dict;
 
-  m_system_cfh = cf_manager->get_or_create_cf(m_db, DEFAULT_SYSTEM_CF_NAME);
+  m_system_cfh =
+      cf_manager->get_or_create_cf(m_db, DEFAULT_SYSTEM_CF_NAME, true);
   rocksdb::ColumnFamilyHandle *default_cfh =
       cf_manager->get_cf(DEFAULT_CF_NAME);