revision-id: fc3d75dfd95df4b772d0804f66483a71d3744efa (fb-prod201903-176-gfc3d75dfd95) parent(s): 703efe24079035200c2e3255ff6e6461cdffb5a0 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-29 16:15:39 +0300 message: Issue #790, MyRocks/MRR: - Increment the rocksdb_rows_read counter. It shows the number of rows that the SQL layer has read. - Remove attempted "optimization" with mrr_values[cur_key].Reset() call. It didn't help anything, actually. --- mysql-test/suite/rocksdb/r/rocksdb_mrr.result | 57 ++++++++++++++++++++++----- mysql-test/suite/rocksdb/t/rocksdb_mrr.test | 47 +++++++++++++++++----- storage/rocksdb/ha_rocksdb.cc | 17 ++++---- 3 files changed, 94 insertions(+), 27 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result index 588a1860923..cb6187fd82a 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result @@ -29,8 +29,9 @@ pk col1 filler a 8 8 8 8 9 9 9 9 # Check the counters -create table t10 as -select * from information_schema.global_status where variable_name like 'rocksdb%multiget%'; +create temporary table t10 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; flush status; select * from t2,t0 where t2.pk=t0.a; pk col1 filler a @@ -47,14 +48,22 @@ pk col1 filler a show status like 'Handler_mrr_init'; Variable_name Value Handler_mrr_init 1 -create table t11 as -select * from information_schema.global_status where variable_name like 'rocksdb%multiget%'; +create temporary table t11 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select variable_name, t11.variable_value - t10.variable_value as DIFF from -t10 join t11 using (VARIABLE_NAME); +t10 join t11 using (VARIABLE_NAME) +having +DIFF>0; variable_name DIFF +ROCKSDB_ROWS_READ 20 +ROCKSDB_NUMBER_DB_NEXT 10 +ROCKSDB_NUMBER_DB_NEXT_FOUND 9 +ROCKSDB_NUMBER_DB_SEEK 1 +ROCKSDB_NUMBER_DB_SEEK_FOUND 1 ROCKSDB_NUMBER_MULTIGET_BYTES_READ 370 ROCKSDB_NUMBER_MULTIGET_GET 1 ROCKSDB_NUMBER_MULTIGET_KEYS_READ 10 @@ -209,7 +218,8 @@ pk1 pk2 col1 filler a 9 9 9 9 9 # Now, run the query and check the counters create temporary table t11 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select * from t3,t0 where t3.col1=t0.a; pk1 pk2 col1 filler a 0 0 0 0 0 @@ -223,7 +233,8 @@ pk1 pk2 col1 filler a 8 8 8 8 8 9 9 9 9 9 create temporary table t12 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select variable_name, t12.variable_value - t11.variable_value as DIFF @@ -232,6 +243,7 @@ t11 join t12 using (VARIABLE_NAME) having DIFF>0; variable_name DIFF +ROCKSDB_ROWS_READ 20 ROCKSDB_NUMBER_DB_NEXT 20 ROCKSDB_NUMBER_DB_NEXT_FOUND 9 ROCKSDB_NUMBER_DB_SEEK 11 @@ -366,12 +378,39 @@ id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY t20 ALL NULL NULL NULL NULL 1 NULL 2 DEPENDENT SUBQUERY t0 ALL NULL NULL NULL NULL 10 NULL 2 DEPENDENT SUBQUERY t2 eq_ref PRIMARY PRIMARY 4 func 1 Using where; Using join buffer (Batched Key Access) -create temporary table t11 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +create temporary table t10 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; flush statistics; select a, a+20 in (select t2.filler from t2,t0 where t2.pk=t0.a+20) from t20; a a+20 in (select t2.filler from t2,t0 where t2.pk=t0.a+20) 1 1 +create temporary table t11 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; +# The below shows ROCKSDB_ROWS_READ=13. +# 1 is from table t20 +# 10 are from table t0, BKA reads all its rows into buffer +# 2 are from table t2. BKA code reads two rows before it figures that +# the subquery has a match and no more rows are needed. +# (note that MultiGet call has read all 10 rows, but SQL layer only has read 2) +select +variable_name, +t11.variable_value - t10.variable_value as DIFF +from +t10 join t11 using (VARIABLE_NAME) +having +DIFF>0; +variable_name DIFF +ROCKSDB_ROWS_READ 13 +ROCKSDB_NUMBER_DB_NEXT 11 +ROCKSDB_NUMBER_DB_NEXT_FOUND 9 +ROCKSDB_NUMBER_DB_SEEK 2 +ROCKSDB_NUMBER_DB_SEEK_FOUND 2 +ROCKSDB_NUMBER_MULTIGET_BYTES_READ 370 +ROCKSDB_NUMBER_MULTIGET_GET 1 +ROCKSDB_NUMBER_MULTIGET_KEYS_READ 10 +drop table t10, t11; # COUNTERS FOR: mrr=ON, primary index lookups select rows_read, rows_requested from information_schema.table_statistics diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test index a882d848c19..0be2a002f1a 100644 --- a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test @@ -26,8 +26,9 @@ select * from t2,t0 where t2.pk=t0.a; select * from t2,t0 where t2.pk=t0.a; --echo # Check the counters -create table t10 as - select * from information_schema.global_status where variable_name like 'rocksdb%multiget%'; +create temporary table t10 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; flush status; # for Handler_mrr_init @@ -35,15 +36,17 @@ select * from t2,t0 where t2.pk=t0.a; show status like 'Handler_mrr_init'; -create table t11 as - select * from information_schema.global_status where variable_name like 'rocksdb%multiget%'; +create temporary table t11 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select variable_name, t11.variable_value - t10.variable_value as DIFF from - t10 join t11 using (VARIABLE_NAME); - + t10 join t11 using (VARIABLE_NAME) +having + DIFF>0; drop table t10, t11; --echo # This will use MRR: @@ -154,12 +157,14 @@ select * from t3,t0 where t3.col1=t0.a; --echo # Now, run the query and check the counters create temporary table t11 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select * from t3,t0 where t3.col1=t0.a; create temporary table t12 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; select variable_name, @@ -239,10 +244,32 @@ set global rocksdb_force_flush_memtable_now=1; explain select a, a+20 in (select t2.filler from t2,t0 where t2.pk=t0.a+20) from t20; -create temporary table t11 as -select * from information_schema.global_status where variable_name like 'rocksdb_num%'; +create temporary table t10 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; flush statistics; + select a, a+20 in (select t2.filler from t2,t0 where t2.pk=t0.a+20) from t20; + +create temporary table t11 as +select * from information_schema.global_status +where variable_name like 'rocksdb_num%' or variable_name='rocksdb_rows_read'; + +--echo # The below shows ROCKSDB_ROWS_READ=13. +--echo # 1 is from table t20 +--echo # 10 are from table t0, BKA reads all its rows into buffer +--echo # 2 are from table t2. BKA code reads two rows before it figures that +--echo # the subquery has a match and no more rows are needed. +--echo # (note that MultiGet call has read all 10 rows, but SQL layer only has read 2) +select + variable_name, + t11.variable_value - t10.variable_value as DIFF +from + t10 join t11 using (VARIABLE_NAME) +having + DIFF>0; +drop table t10, t11; + --echo # COUNTERS FOR: mrr=ON, primary index lookups query_vertical select rows_read, rows_requested diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index bbdaf9c9976..4101d1255f5 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -15509,12 +15509,7 @@ void ha_rocksdb::mrr_free() { } void ha_rocksdb::mrr_free_rows() { - ssize_t n_pinned = 0; for (ssize_t i = 0; i < mrr_n_elements; i++) { - if (mrr_values[i].IsPinned()) { - n_pinned++; - mrr_values[i].Reset(); - } mrr_values[i].~PinnableSlice(); mrr_statuses[i].~Status(); // no need to free mrr_keys @@ -15525,7 +15520,9 @@ void ha_rocksdb::mrr_free_rows() { // Count them in in "rows_read" anyway. (This is only necessary when using // clustered PK. When using a secondary key, the index-only part of the scan // that collects the rowids has caused all counters to be incremented) - if (mrr_used_cpk) stats.rows_read += n_pinned; + if (mrr_used_cpk && mrr_n_elements) { + stats.rows_read += mrr_n_elements - mrr_read_index; + } mrr_n_elements = 0; // We can't rely on the data from HANDLER_BUFFER once the scan is over, so: @@ -15583,7 +15580,6 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { m_retrieved_record.Reset(); m_retrieved_record.PinSlice(mrr_values[cur_key], &mrr_values[cur_key]); - mrr_values[cur_key].Reset(); /* If we found the record, but it's expired, pretend we didn't find it. */ if (m_pk_descr->has_ttl() && @@ -15593,8 +15589,13 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { } rc = convert_record_from_storage_format(&rowkey, table->record[0]); - if (active_index == table->s->primary_key) + + // When using a secondary index, the scan on secondary index increments the + // count + if (active_index == table->s->primary_key) { stats.rows_read++; + update_row_stats(ROWS_READ); + } break; } table->status = rc ? STATUS_NOT_FOUND : 0;