Add parameter for calculate_stats function of the indexes to recalculate only + testing

Ahmed Salama · Ahmed Salama · commit 5941e5517a54 · 2018-03-14T16:47:47.000-07:00
Summary: Change calculate_stats() to recalculate statistics for certain set of indexes only; Those that changed. Test Plan: Added new test for asserting index statistics Used gdb to see the new check changing the control flow (some indexes skip and some recalculate). Reviewers: shashanktyagi, mung Differential Revision: https://phabricator.intern.facebook.com/D7075934 Tasks: T15057256
diff --git a/mysql-test/suite/rocksdb/r/add_index_inplace.result b/mysql-test/suite/rocksdb/r/add_index_inplace.result
@@ -445,3 +445,24 @@ t1	CREATE TABLE `t1` (
   KEY `kb` (`b`(8))
 ) ENGINE=ROCKSDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin
 DROP TABLE t1;
+SET @prior_rocksdb_table_stats_sampling_pct = @@rocksdb_table_stats_sampling_pct;
+set global rocksdb_table_stats_sampling_pct = 100;
+CREATE TABLE t1 (a INT, b INT, PRIMARY KEY ka(a)) ENGINE=RocksDB;
+INSERT INTO t1 (a, b) VALUES (1, 10);
+INSERT INTO t1 (a, b) VALUES (2, 10);
+INSERT INTO t1 (a, b) VALUES (3, 20);
+INSERT INTO t1 (a, b) VALUES (4, 20);
+set global rocksdb_force_flush_memtable_now=1;
+analyze table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+SHOW INDEX in t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment	Index_comment
+t1	0	PRIMARY	1	a	A	4	NULL	NULL		LSMTREE		
+ALTER TABLE t1 ADD INDEX kb(b), ALGORITHM=INPLACE;
+SHOW INDEX in t1;
+Table	Non_unique	Key_name	Seq_in_index	Column_name	Collation	Cardinality	Sub_part	Packed	Null	Index_type	Comment	Index_comment
+t1	0	PRIMARY	1	a	A	4	NULL	NULL		LSMTREE		
+t1	1	kb	1	b	A	2	NULL	NULL	YES	LSMTREE		
+DROP TABLE t1;
+SET global rocksdb_table_stats_sampling_pct = @prior_rocksdb_table_stats_sampling_pct;
diff --git a/mysql-test/suite/rocksdb/t/add_index_inplace.test b/mysql-test/suite/rocksdb/t/add_index_inplace.test
@@ -383,6 +383,24 @@ if ($end_max_index_id <= $start_max_index_id) {
 SHOW CREATE TABLE t1;
 DROP TABLE t1;
 
+# Cardinality checks for indexes statistics
+SET @prior_rocksdb_table_stats_sampling_pct = @@rocksdb_table_stats_sampling_pct;
+set global rocksdb_table_stats_sampling_pct = 100;
 
+CREATE TABLE t1 (a INT, b INT, PRIMARY KEY ka(a)) ENGINE=RocksDB;
 
+INSERT INTO t1 (a, b) VALUES (1, 10);
+INSERT INTO t1 (a, b) VALUES (2, 10);
+INSERT INTO t1 (a, b) VALUES (3, 20);
+INSERT INTO t1 (a, b) VALUES (4, 20);
 
+set global rocksdb_force_flush_memtable_now=1;
+analyze table t1;
+
+SHOW INDEX in t1;
+
+ALTER TABLE t1 ADD INDEX kb(b), ALGORITHM=INPLACE;
+SHOW INDEX in t1;
+
+DROP TABLE t1;
+SET global rocksdb_table_stats_sampling_pct = @prior_rocksdb_table_stats_sampling_pct;
diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc
@@ -10942,24 +10942,34 @@ int ha_rocksdb::optimize(THD *const thd, HA_CHECK_OPT *const check_opt) {
   DBUG_RETURN(HA_EXIT_SUCCESS);
 }
 
-int ha_rocksdb::calculate_stats(const TABLE *const table_arg, THD *const thd,
-                                HA_CHECK_OPT *const check_opt) {
+int ha_rocksdb::calculate_stats(
+    const TABLE *const table_arg,
+    const std::unordered_set<GL_INDEX_ID> &to_recalc) {
   DBUG_ENTER_FUNC();
 
   // find per column family key ranges which need to be queried
   std::unordered_map<rocksdb::ColumnFamilyHandle *, std::vector<rocksdb::Range>>
       ranges;
-  std::unordered_set<GL_INDEX_ID> ids_to_check;
+  std::unordered_map<GL_INDEX_ID, Rdb_index_stats> stats;
+  std::vector<uint> to_recalc_indexes;
   std::vector<uchar> buf(table_arg->s->keys * 2 *
                          Rdb_key_def::INDEX_NUMBER_SIZE);
-  std::unordered_map<GL_INDEX_ID, Rdb_index_stats> stats;
   for (uint i = 0; i < table_arg->s->keys; i++) {
+    const Rdb_key_def &kd = *m_key_descr_arr[i];
+    const GL_INDEX_ID index_id = kd.get_gl_index_id();
+    if (to_recalc.find(index_id) == to_recalc.end()) {
+      continue;
+    }
+
+    to_recalc_indexes.push_back(i);
+  }
+
+  for (uint i : to_recalc_indexes) {
     const auto bufp = &buf[i * 2 * Rdb_key_def::INDEX_NUMBER_SIZE];
     const Rdb_key_def &kd = *m_key_descr_arr[i];
     const GL_INDEX_ID index_id = kd.get_gl_index_id();
     ranges[kd.get_cf()].push_back(get_range(i, bufp));
 
-    ids_to_check.insert(index_id);
     // Initialize the stats to 0. If there are no files that contain
     // this gl_index_id, then 0 should be stored for the cached stats.
     stats[index_id] = Rdb_index_stats(index_id);
@@ -10997,8 +11007,9 @@ int ha_rocksdb::calculate_stats(const TABLE *const table_arg, THD *const thd,
         other SQL tables, it can be that we're only seeing a small fraction
         of table's entries (and so we can't update statistics based on that).
       */
-      if (ids_to_check.find(it1.m_gl_index_id) == ids_to_check.end())
+      if (to_recalc.find(it1.m_gl_index_id) == to_recalc.end()) {
         continue;
+      }
 
       auto kd = ddl_manager.safe_find(it1.m_gl_index_id);
       DBUG_ASSERT(kd != nullptr);
@@ -11011,8 +11022,9 @@ int ha_rocksdb::calculate_stats(const TABLE *const table_arg, THD *const thd,
   Rdb_tbl_card_coll cardinality_collector(rocksdb_table_stats_sampling_pct);
   auto read_opts = rocksdb::ReadOptions();
   read_opts.read_tier = rocksdb::ReadTier::kMemtableTier;
-  for (uint i = 0; i < table_arg->s->keys; i++) {
+  for (const uint i : to_recalc_indexes) {
     const Rdb_key_def &kd = *m_key_descr_arr[i];
+
     Rdb_index_stats &stat = stats[kd.get_gl_index_id()];
 
     uchar r_buf[Rdb_key_def::INDEX_NUMBER_SIZE * 2];
@@ -11069,8 +11081,18 @@ int ha_rocksdb::calculate_stats(const TABLE *const table_arg, THD *const thd,
 int ha_rocksdb::analyze(THD *const thd, HA_CHECK_OPT *const check_opt) {
   DBUG_ENTER_FUNC();
 
-  if (table && calculate_stats(table, thd, check_opt) != HA_EXIT_SUCCESS) {
-    DBUG_RETURN(HA_ADMIN_FAILED);
+  if (table) {
+    std::unordered_set<GL_INDEX_ID> ids_to_check;
+    for (uint i = 0; i < table->s->keys; i++) {
+      const Rdb_key_def &kd = *m_key_descr_arr[i];
+      const GL_INDEX_ID index_id = kd.get_gl_index_id();
+      ids_to_check.insert(index_id);
+    }
+
+    int res = calculate_stats(table, ids_to_check);
+    if (res != HA_EXIT_SUCCESS) {
+      DBUG_RETURN(HA_ADMIN_FAILED);
+    }
   }
 
   DBUG_RETURN(HA_ADMIN_OK);
@@ -11935,7 +11957,7 @@ bool ha_rocksdb::commit_inplace_alter_table(
       prevents us from updating the stats normally as the ddl_manager cannot
       find the proper gl_index_ids yet during adjust_stats calls.
     */
-    if (calculate_stats(altered_table, nullptr, nullptr)) {
+    if (calculate_stats(altered_table, create_index_ids)) {
       /* Failed to update index statistics, should never happen */
       DBUG_ASSERT(0);
     }
diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h
@@ -1327,8 +1327,8 @@ class ha_rocksdb : public my_core::handler {
       MY_ATTRIBUTE((__warn_unused_result__));
   int analyze(THD *const thd, HA_CHECK_OPT *const check_opt) override
       MY_ATTRIBUTE((__warn_unused_result__));
-  int calculate_stats(const TABLE *const table_arg, THD *const thd,
-                      HA_CHECK_OPT *const check_opt)
+  int calculate_stats(const TABLE *const table_arg,
+                      const std::unordered_set<GL_INDEX_ID> &to_recalc)
       MY_ATTRIBUTE((__warn_unused_result__));
 
   enum_alter_inplace_result check_if_supported_inplace_alter(
