- commits - lists.mariadb.org

[Commits] fc3d75dfd95: Issue #790, MyRocks/MRR:
by Sergei Petrunia 29 Oct '19

29 Oct '19

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;

1 0

[Commits] 703efe24079: Cleanup tests
by Sergei Petrunia 29 Oct '19

29 Oct '19

revision-id: 703efe24079035200c2e3255ff6e6461cdffb5a0 (fb-prod201903-175-g703efe24079) parent(s): 441a8905fa0c94b7c8b9a996c799b460553c1d66 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-29 13:32:34 +0300 message: Cleanup tests --- mysql-test/suite/rocksdb/r/rocksdb_mrr.result | 3 --- mysql-test/suite/rocksdb/r/rocksdb_mrr2.result | 1 - mysql-test/suite/rocksdb/t/rocksdb_mrr.test | 1 - 3 files changed, 5 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result index 465966c5b6e..588a1860923 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result @@ -243,9 +243,6 @@ drop table t11,t12; # # Check the userstat counters # -show variables like '%user_table_stat%'; -Variable_name Value -user_table_stats_control OFF set optimizer_switch='mrr=off'; flush statistics; select * from t3,t0 where t3.col1=t0.a; diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result index 5628f1bf06d..a01a33b9ae8 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result @@ -1,4 +1,3 @@ -drop table if exists t0,t1,t2; show variables like 'rocksdb_block_cache_size%'; Variable_name Value rocksdb_block_cache_size 1024 diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test index 9b94cb3cbe9..a882d848c19 100644 --- a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test @@ -174,7 +174,6 @@ drop table t11,t12; --echo # --echo # Check the userstat counters --echo # -show variables like '%user_table_stat%'; set optimizer_switch='mrr=off'; flush statistics;

1 0

[Commits] 441a8905fa0: Issue #790, MyRocks/MRR: properly increment per-table read counters
by Sergei Petrunia 28 Oct '19

28 Oct '19

revision-id: 441a8905fa0c94b7c8b9a996c799b460553c1d66 (fb-prod201903-174-g441a8905fa0) parent(s): 2527cd79440e2145eca25c7f8735ad1c8b8c4f3b author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-28 19:56:59 +0300 message: Issue #790, MyRocks/MRR: properly increment per-table read counters - Properly increment I_S.table_statistics.rows_* counters - Do release the PinnableSlice that's holding the retrieved record as soon as we don't need it (before, we released all records in ha_rocksdb::mrr_free_rows(). Now, we only release them there if the mrr scan was not finished). - Clean up in testcases --- mysql-test/suite/rocksdb/r/rocksdb_mrr.result | 148 +++++++++++++++++++++++++- mysql-test/suite/rocksdb/t/rocksdb_mrr.test | 92 ++++++++++++++-- mysql-test/suite/rocksdb/t/rocksdb_mrr2.test | 4 - storage/rocksdb/ha_rocksdb.cc | 23 +++- storage/rocksdb/ha_rocksdb.h | 1 + 5 files changed, 254 insertions(+), 14 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result index 6aa2ff4dc23..465966c5b6e 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_mrr.result +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr.result @@ -1,4 +1,3 @@ -drop table if exists t0,t1,t2,t3,t4,t5,t6,t7,t8,t9,t10,t11; create table t0(a int); insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); create table t1(a int); @@ -241,8 +240,151 @@ ROCKSDB_NUMBER_MULTIGET_BYTES_READ 370 ROCKSDB_NUMBER_MULTIGET_GET 1 ROCKSDB_NUMBER_MULTIGET_KEYS_READ 10 drop table t11,t12; -# Same as above but for MRR (not BKA): -# This will not use MRR: +# +# Check the userstat counters +# +show variables like '%user_table_stat%'; +Variable_name Value +user_table_stats_control OFF +set optimizer_switch='mrr=off'; +flush statistics; +select * from t3,t0 where t3.col1=t0.a; +pk1 pk2 col1 filler a +0 0 0 0 0 +1 1 1 1 1 +2 2 2 2 2 +3 3 3 3 3 +4 4 4 4 4 +5 5 5 5 5 +6 6 6 6 6 +7 7 7 7 7 +8 8 8 8 8 +9 9 9 9 9 +# COUNTERS FOR: mrr=OFF, secondary index lookups +select +rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from +information_schema.table_statistics +where table_name = 't3'; +rows_inserted 0 +rows_updated 0 +rows_deleted 0 +rows_read 10 +rows_requested 20 +set optimizer_switch='mrr=on'; +flush statistics; +select * from t3,t0 where t3.col1=t0.a; +pk1 pk2 col1 filler a +0 0 0 0 0 +1 1 1 1 1 +2 2 2 2 2 +3 3 3 3 3 +4 4 4 4 4 +5 5 5 5 5 +6 6 6 6 6 +7 7 7 7 7 +8 8 8 8 8 +9 9 9 9 9 +# COUNTERS FOR: mrr=ON, secondary index lookups +select +rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from +information_schema.table_statistics +where table_name = 't3'; +rows_inserted 0 +rows_updated 0 +rows_deleted 0 +rows_read 10 +rows_requested 20 +set optimizer_switch='mrr=off'; +flush statistics; +explain +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t0 ALL NULL NULL NULL NULL 10 Using where +1 SIMPLE t3 eq_ref PRIMARY PRIMARY 8 test.t0.a,test.t0.a 1 NULL +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +pk1 pk2 col1 filler a +0 0 0 0 0 +1 1 1 1 1 +2 2 2 2 2 +3 3 3 3 3 +4 4 4 4 4 +5 5 5 5 5 +6 6 6 6 6 +7 7 7 7 7 +8 8 8 8 8 +9 9 9 9 9 +# COUNTERS FOR: mrr=OFF, primary index lookups +select +rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from +information_schema.table_statistics +where table_name = 't3'; +rows_inserted 0 +rows_updated 0 +rows_deleted 0 +rows_read 10 +rows_requested 10 +set optimizer_switch='mrr=on'; +flush statistics; +explain +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t0 ALL NULL NULL NULL NULL 10 Using where +1 SIMPLE t3 eq_ref PRIMARY PRIMARY 8 test.t0.a,test.t0.a 1 Using join buffer (Batched Key Access) +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +pk1 pk2 col1 filler a +0 0 0 0 0 +1 1 1 1 1 +2 2 2 2 2 +3 3 3 3 3 +4 4 4 4 4 +5 5 5 5 5 +6 6 6 6 6 +7 7 7 7 7 +8 8 8 8 8 +9 9 9 9 9 +# COUNTERS FOR: mrr=ON, primary index lookups +select +rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from +information_schema.table_statistics +where table_name = 't3'; +rows_inserted 0 +rows_updated 0 +rows_deleted 0 +rows_read 10 +rows_requested 10 +# +# Check how the counters are incremented when SQL +# layer doesn't read all of the MultiGet results +# +create table t20 (a int); +insert into t20 values (1); +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; +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%'; +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 +# COUNTERS FOR: mrr=ON, primary index lookups +select rows_read, rows_requested +from information_schema.table_statistics +where table_name = 't2'; +rows_read 10 +rows_requested 10 +drop table t20; +# +# Check how MRR works without BKA +# explain select t3.col1 from t3 where t3.col1=20 or t3.col1 between 25 and 28; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t3 range col1 col1 5 NULL 2 Using where; Using index diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test index 74675b1cb5b..9b94cb3cbe9 100644 --- a/mysql-test/suite/rocksdb/t/rocksdb_mrr.test +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr.test @@ -1,9 +1,8 @@ +# +# Test for MyRocks' MRR implementation based on MultiGet. +# --source include/have_rocksdb.inc ---disable_warnings -drop table if exists t0,t1,t2,t3,t4,t5,t6,t7,t8,t9,t10,t11; ---enable_warnings - create table t0(a int); insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); @@ -172,8 +171,89 @@ having drop table t11,t12; ---echo # Same as above but for MRR (not BKA): ---echo # This will not use MRR: +--echo # +--echo # Check the userstat counters +--echo # +show variables like '%user_table_stat%'; +set optimizer_switch='mrr=off'; +flush statistics; + +select * from t3,t0 where t3.col1=t0.a; + +--echo # COUNTERS FOR: mrr=OFF, secondary index lookups +query_vertical +select + rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from + information_schema.table_statistics +where table_name = 't3'; + +set optimizer_switch='mrr=on'; +flush statistics; + +select * from t3,t0 where t3.col1=t0.a; + +--echo # COUNTERS FOR: mrr=ON, secondary index lookups +query_vertical +select + rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from + information_schema.table_statistics +where table_name = 't3'; + +set optimizer_switch='mrr=off'; +flush statistics; +explain +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; + +--echo # COUNTERS FOR: mrr=OFF, primary index lookups +query_vertical +select + rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from + information_schema.table_statistics +where table_name = 't3'; + +set optimizer_switch='mrr=on'; +flush statistics; +explain +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; +select * from t3,t0 where t3.pk1=t0.a and t3.pk2=t0.a; + +--echo # COUNTERS FOR: mrr=ON, primary index lookups +query_vertical +select + rows_inserted, rows_updated, rows_deleted, rows_read, rows_requested +from + information_schema.table_statistics +where table_name = 't3'; + +--echo # +--echo # Check how the counters are incremented when SQL +--echo # layer doesn't read all of the MultiGet results +--echo # +create table t20 (a int); +insert into t20 values (1); +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%'; +flush statistics; +select a, a+20 in (select t2.filler from t2,t0 where t2.pk=t0.a+20) from t20; +--echo # COUNTERS FOR: mrr=ON, primary index lookups +query_vertical +select rows_read, rows_requested +from information_schema.table_statistics +where table_name = 't2'; + +drop table t20; +--echo # +--echo # Check how MRR works without BKA +--echo # explain select t3.col1 from t3 where t3.col1=20 or t3.col1 between 25 and 28; --echo # This will use MRR: diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test index f9ec2fe518e..8e747b3cfd4 100644 --- a/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test @@ -5,10 +5,6 @@ # --source include/have_rocksdb.inc ---disable_warnings -drop table if exists t0,t1,t2; ---enable_warnings - show variables like 'rocksdb_block_cache_size%'; create table t0(a int); insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index e32d1bfa93f..bbdaf9c9976 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -6218,6 +6218,7 @@ ha_rocksdb::ha_rocksdb(my_core::handlerton *const hton, mrr_rowid_reader(nullptr), mrr_n_elements(0), mrr_enabled_keyread(false), + mrr_used_cpk(false), m_in_rpl_delete_rows(false), m_in_rpl_update_rows(false), m_force_skip_unique_check(false) {} @@ -15340,9 +15341,11 @@ int ha_rocksdb::multi_range_read_init(RANGE_SEQ_IF *seq, void *seq_init_param, // ICP is not supported for PK, so we don't expect that BKA's variant // of ICP would be used: DBUG_ASSERT(!mrr_funcs.skip_index_tuple); + mrr_used_cpk = true; mrr_rowid_reader = new Mrr_pk_scan_rowid_source(this, seq_init_param, n_ranges, mode); } else { + mrr_used_cpk = false; auto reader = new Mrr_sec_key_rowid_source(this); reader->init(seq, seq_init_param, n_ranges, mode); mrr_rowid_reader = reader; @@ -15485,6 +15488,9 @@ int ha_rocksdb::mrr_fill_buffer() { Rdb_transaction *const tx = get_or_create_tx(table->in_use); + if (active_index == table->s->primary_key) + stats.rows_requested += mrr_n_elements; + tx->multi_get(m_pk_descr->get_cf(), mrr_n_elements, mrr_keys, mrr_values, mrr_statuses, mrr_sorted_mode); @@ -15503,12 +15509,24 @@ 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++) { - mrr_values[i].Reset(); + 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 } + + // There could be rows that MultiGet has returned but MyRocks hasn't + // returned to the SQL layer (typically due to LIMIT clause) + // 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; + mrr_n_elements = 0; // We can't rely on the data from HANDLER_BUFFER once the scan is over, so: mrr_values = nullptr; @@ -15565,6 +15583,7 @@ 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() && @@ -15574,6 +15593,8 @@ 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) + stats.rows_read++; break; } table->status = rc ? STATUS_NOT_FOUND : 0; diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index 6d3fa75e3fa..0290a80637b 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/storage/rocksdb/ha_rocksdb.h @@ -692,6 +692,7 @@ class ha_rocksdb : public my_core::handler { // if true, MRR code has enabled keyread (and should disable it back) bool mrr_enabled_keyread; + bool mrr_used_cpk; int mrr_fill_buffer(); void mrr_free_rows();

1 0

[Commits] 34a3874b533: MDEV-20424: New default value for optimizer_use_condition-selectivity leads to bad plan
by Varun 26 Oct '19

26 Oct '19

revision-id: 34a3874b53324234e8e6bf70ea104de85ba6cf05 (mariadb-10.1.41-63-g34a3874b533) parent(s): a41d429765c7ddb528b9b438c68b25ff55d3bd55 author: Varun Gupta committer: Varun Gupta timestamp: 2019-10-27 02:25:07 +0530 message: MDEV-20424: New default value for optimizer_use_condition-selectivity leads to bad plan In the function prev_record_reads where one finds the different row combinations for a subset of partial join, it did not take into account the selectivity of tables involved in the subset of partial join. --- mysql-test/r/selectivity.result | 57 +++++++++++++++++++++++++++++++++++++++++ mysql-test/t/selectivity.test | 29 +++++++++++++++++++++ sql/sql_select.cc | 3 +++ 3 files changed, 89 insertions(+) diff --git a/mysql-test/r/selectivity.result b/mysql-test/r/selectivity.result index d0bbb46cb0a..5fe6986e9ff 100644 --- a/mysql-test/r/selectivity.result +++ b/mysql-test/r/selectivity.result @@ -1753,4 +1753,61 @@ a 1991 set optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivity; DROP TABLE t1; +# +# MDEV-20424: New default value for optimizer_use_condition-selectivity +# leads to bad plan +# +create table t1(a int, b int, c int, d int, key(a,b)); +insert into t1 select 50,seq-1,seq-1,seq from seq_1_to_10; +insert into t1 select seq-1,seq-1,seq-1,seq from seq_1_to_100 limit 90; +create table t2(a int, b int, c int, primary key(a)); +insert into t2 select seq-1,seq-1,seq-1 from seq_1_to_100; +create table t3(a int, b int, c int, primary key(a)); +insert into t3 select seq-1,seq-1,seq-1 from seq_1_to_100 limit 30; +set optimizer_use_condition_selectivity=1; +explain extended select t1.b,t2.a,t3.a,t3.b from t1,t2,t3 +where t1.c = t2.a AND t1.d = t3.a and t1.a = 50 and t1.b <= 100; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 range a a 10 NULL 9 100.00 Using index condition; Using where +1 SIMPLE t3 eq_ref PRIMARY PRIMARY 4 test.t1.d 1 100.00 +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t1.c 1 100.00 Using index +Warnings: +Note 1003 select `test`.`t1`.`b` AS `b`,`test`.`t2`.`a` AS `a`,`test`.`t3`.`a` AS `a`,`test`.`t3`.`b` AS `b` from `test`.`t1` join `test`.`t2` join `test`.`t3` where ((`test`.`t2`.`a` = `test`.`t1`.`c`) and (`test`.`t3`.`a` = `test`.`t1`.`d`) and (`test`.`t1`.`a` = 50) and (`test`.`t1`.`b` <= 100)) +select t1.b,t2.a,t3.a,t3.b from t1,t2,t3 +where t1.c = t2.a AND t1.d = t3.a and t1.a = 50 and t1.b <= 100; +b a a b +0 0 1 1 +1 1 2 2 +2 2 3 3 +3 3 4 4 +4 4 5 5 +5 5 6 6 +6 6 7 7 +7 7 8 8 +8 8 9 9 +9 9 10 10 +set optimizer_use_condition_selectivity=2; +explain extended select t1.b,t2.a,t3.a,t3.b from t1,t2,t3 +where t1.c = t2.a AND t1.d = t3.a and t1.a = 50 and t1.b <= 100; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 range a a 10 NULL 9 9.00 Using index condition; Using where +1 SIMPLE t3 eq_ref PRIMARY PRIMARY 4 test.t1.d 1 100.00 +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 test.t1.c 1 100.00 Using index +Warnings: +Note 1003 select `test`.`t1`.`b` AS `b`,`test`.`t2`.`a` AS `a`,`test`.`t3`.`a` AS `a`,`test`.`t3`.`b` AS `b` from `test`.`t1` join `test`.`t2` join `test`.`t3` where ((`test`.`t2`.`a` = `test`.`t1`.`c`) and (`test`.`t3`.`a` = `test`.`t1`.`d`) and (`test`.`t1`.`a` = 50) and (`test`.`t1`.`b` <= 100)) +select t1.b,t2.a,t3.a,t3.b from t1,t2,t3 +where t1.c = t2.a AND t1.d = t3.a and t1.a = 50 and t1.b <= 100; +b a a b +0 0 1 1 +1 1 2 2 +2 2 3 3 +3 3 4 4 +4 4 5 5 +5 5 6 6 +6 6 7 7 +7 7 8 8 +8 8 9 9 +9 9 10 10 +set optimizer_use_condition_selectivity= @@optimizer_use_condition_selectivity; +drop table t1,t2,t3; # End of 10.1 tests diff --git a/mysql-test/t/selectivity.test b/mysql-test/t/selectivity.test index 0deacc390db..a93ad5efb07 100644 --- a/mysql-test/t/selectivity.test +++ b/mysql-test/t/selectivity.test @@ -1205,5 +1205,34 @@ set optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivit DROP TABLE t1; +--echo # +--echo # MDEV-20424: New default value for optimizer_use_condition-selectivity +--echo # leads to bad plan +--echo # + +create table t1(a int, b int, c int, d int, key(a,b)); +insert into t1 select 50,seq-1,seq-1,seq from seq_1_to_10; +insert into t1 select seq-1,seq-1,seq-1,seq from seq_1_to_100 limit 90; + +create table t2(a int, b int, c int, primary key(a)); +insert into t2 select seq-1,seq-1,seq-1 from seq_1_to_100; + +create table t3(a int, b int, c int, primary key(a)); +insert into t3 select seq-1,seq-1,seq-1 from seq_1_to_100 limit 30; + +let $query= select t1.b,t2.a,t3.a,t3.b from t1,t2,t3 +where t1.c = t2.a AND t1.d = t3.a and t1.a = 50 and t1.b <= 100; + +set optimizer_use_condition_selectivity=1; +eval explain extended $query; +eval $query; + +set optimizer_use_condition_selectivity=2; +eval explain extended $query; +eval $query; +set optimizer_use_condition_selectivity= @@optimizer_use_condition_selectivity; + +drop table t1,t2,t3; + --echo # End of 10.1 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c6e70c2430c..dfc9f729118 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -8470,7 +8470,10 @@ prev_record_reads(POSITION *positions, uint idx, table_map found_ref) #max_nested_outer_joins=64-1) will not make it any more precise. */ if (pos->records_read) + { found= COST_MULT(found, pos->records_read); + found*= pos->cond_selectivity; + } } } return found;

1 0

[Commits] 2527cd79440: Issue #790, MyRocks/MRR: add test coverage for previous cset
by psergey 21 Oct '19

21 Oct '19

revision-id: 2527cd79440e2145eca25c7f8735ad1c8b8c4f3b (fb-prod201903-173-g2527cd79440) parent(s): 24860d7505161c15a1d4b3b76519dbcfa0298c2c author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-21 23:05:47 +0300 message: Issue #790, MyRocks/MRR: add test coverage for previous cset --- mysql-test/suite/rocksdb/r/rocksdb_mrr2.result | 39 ++++++++++++++++++++ mysql-test/suite/rocksdb/t/rocksdb_mrr2-master.opt | 1 + mysql-test/suite/rocksdb/t/rocksdb_mrr2.test | 41 ++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result b/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result new file mode 100644 index 00000000000..5628f1bf06d --- /dev/null +++ b/mysql-test/suite/rocksdb/r/rocksdb_mrr2.result @@ -0,0 +1,39 @@ +drop table if exists t0,t1,t2; +show variables like 'rocksdb_block_cache_size%'; +Variable_name Value +rocksdb_block_cache_size 1024 +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t1(a int); +insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C; +create table t2 ( +pk int primary key, +col1 int, +blob1 text, +blob2 text +) engine=rocksdb; +insert into t2 select a,a,repeat(a,100), repeat(a,200) from t1; +set global rocksdb_force_flush_memtable_now=1; +set @save_optimizer_switch=@@optimizer_switch; +set optimizer_switch='mrr=on,mrr_cost_based=off,batched_key_access=on'; +explain +select pk, col1 from t2,t0 +where t2.pk=t0.a*90 and md5(blob1)=md5(repeat(t2.pk,100)) and md5(blob2)=md5(repeat(t2.pk,200)); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t0 ALL NULL NULL NULL NULL 10 NULL +1 SIMPLE t2 eq_ref PRIMARY PRIMARY 4 func 1 Using where; Using join buffer (Batched Key Access) +select pk, col1 from t2,t0 +where t2.pk=t0.a*90 and md5(blob1)=md5(repeat(t2.pk,100)) and md5(blob2)=md5(repeat(t2.pk,200)); +pk col1 +0 0 +90 90 +180 180 +270 270 +360 360 +450 450 +540 540 +630 630 +720 720 +810 810 +set optimizer_switch=@save_optimizer_switch; +drop table t0,t1,t2; diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr2-master.opt b/mysql-test/suite/rocksdb/t/rocksdb_mrr2-master.opt new file mode 100644 index 00000000000..08fa393d3c3 --- /dev/null +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr2-master.opt @@ -0,0 +1 @@ +--rocksdb-block-cache-size=1024 diff --git a/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test b/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test new file mode 100644 index 00000000000..f9ec2fe518e --- /dev/null +++ b/mysql-test/suite/rocksdb/t/rocksdb_mrr2.test @@ -0,0 +1,41 @@ +# +# Test if MyRocks/MRR correctly releases PinnableSlice objects with value data +# (if it doesn't, this test will produce valgrind errors: invalid memory reads +# inside my_md5_hash) +# +--source include/have_rocksdb.inc + +--disable_warnings +drop table if exists t0,t1,t2; +--enable_warnings + +show variables like 'rocksdb_block_cache_size%'; +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); + +create table t1(a int); +insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C; + +create table t2 ( + pk int primary key, + col1 int, + blob1 text, + blob2 text +) engine=rocksdb; + +insert into t2 select a,a,repeat(a,100), repeat(a,200) from t1; + +set global rocksdb_force_flush_memtable_now=1; + +set @save_optimizer_switch=@@optimizer_switch; +set optimizer_switch='mrr=on,mrr_cost_based=off,batched_key_access=on'; + +explain +select pk, col1 from t2,t0 +where t2.pk=t0.a*90 and md5(blob1)=md5(repeat(t2.pk,100)) and md5(blob2)=md5(repeat(t2.pk,200)); + +select pk, col1 from t2,t0 +where t2.pk=t0.a*90 and md5(blob1)=md5(repeat(t2.pk,100)) and md5(blob2)=md5(repeat(t2.pk,200)); +set optimizer_switch=@save_optimizer_switch; + +drop table t0,t1,t2;

1 0

[Commits] 24860d75051: Issue #790, MyRocks/MRR: address more review input: don't release m_retrieved_record
by Sergei Petrunia 21 Oct '19

21 Oct '19

revision-id: 24860d7505161c15a1d4b3b76519dbcfa0298c2c (fb-prod201903-172-g24860d75051) parent(s): caaf170ba4210bf614140d88c0fa108dff063b0d author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-21 19:42:18 +0200 message: Issue #790, MyRocks/MRR: address more review input: don't release m_retrieved_record Keep the m_retrieved_record pinned. --- storage/rocksdb/ha_rocksdb.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 1ae5c1d6989..e32d1bfa93f 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -15570,14 +15570,10 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { if (m_pk_descr->has_ttl() && should_hide_ttl_rec(*m_pk_descr, m_retrieved_record, tx->m_snapshot_timestamp)) { - m_retrieved_record.Reset(); - mrr_values[cur_key].Reset(); continue; } rc = convert_record_from_storage_format(&rowkey, table->record[0]); - m_retrieved_record.Reset(); - mrr_values[cur_key].Reset(); break; } table->status = rc ? STATUS_NOT_FOUND : 0;

1 0

[Commits] 7546be4493e: Support Create_time and Update_time in MyRocks table status
by psergey 16 Oct '19

16 Oct '19

revision-id: 7546be4493efb8509aae772e02d382078db8c5e7 (fb-prod201903-145-g7546be4493e) parent(s): 6dc3fa2e054d999a12cd5d7aaf637e4ac7c3f149 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-17 01:08:00 +0300 message: Support Create_time and Update_time in MyRocks table status Address review input: - Change the default to be rocksdb_table_dictionary_format=1. One needs to set the parameter to 2 to get Create_time to be saved. - Other small changes --- mysql-test/suite/rocksdb/r/rocksdb.result | 2 +- .../r/rocksdb_table_dictionary_format.result | 2 +- .../suite/rocksdb/r/show_table_status.result | 14 ++++++++++ mysql-test/suite/rocksdb/t/show_table_status.test | 25 +++++++++++++++-- .../r/rocksdb_table_dictionary_format_basic.result | 2 +- storage/rocksdb/rdb_datadic.cc | 32 ++++++++++++++-------- storage/rocksdb/rdb_datadic.h | 16 +++++++---- 7 files changed, 71 insertions(+), 22 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/rocksdb.result b/mysql-test/suite/rocksdb/r/rocksdb.result index c76f31afaab..59856d2f6e8 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb.result +++ b/mysql-test/suite/rocksdb/r/rocksdb.result @@ -979,7 +979,7 @@ rocksdb_store_row_debug_checksums OFF rocksdb_strict_collation_check OFF rocksdb_strict_collation_exceptions rocksdb_table_cache_numshardbits 6 -rocksdb_table_dictionary_format 2 +rocksdb_table_dictionary_format 1 rocksdb_table_stats_background_thread_nice_value 19 rocksdb_table_stats_max_num_rows_scanned 0 rocksdb_table_stats_recalc_threshold_count 100 diff --git a/mysql-test/suite/rocksdb/r/rocksdb_table_dictionary_format.result b/mysql-test/suite/rocksdb/r/rocksdb_table_dictionary_format.result index 8f2fa96a04a..82f1064f937 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_table_dictionary_format.result +++ b/mysql-test/suite/rocksdb/r/rocksdb_table_dictionary_format.result @@ -1,6 +1,6 @@ select @@rocksdb_table_dictionary_format; @@rocksdb_table_dictionary_format -2 +1 # # Server restarted # diff --git a/mysql-test/suite/rocksdb/r/show_table_status.result b/mysql-test/suite/rocksdb/r/show_table_status.result index 345882040ef..58119860482 100644 --- a/mysql-test/suite/rocksdb/r/show_table_status.result +++ b/mysql-test/suite/rocksdb/r/show_table_status.result @@ -25,6 +25,9 @@ DROP DATABASE `db_new..............................................end`; # # MDEV-17171: Bug: RocksDB Tables do not have "Creation Date" # +select @@rocksdb_table_dictionary_format; +@@rocksdb_table_dictionary_format +2 use test; create table t1 (a int) engine=rocksdb; select create_time is not null, update_time, check_time @@ -99,3 +102,14 @@ where table_schema=database() and table_name='t1'; create_time=(select create_time from t2) 1 update_time NULL drop table t1, t2; +# +# Check how it works for partitioned tables +# +create table t1 (pk int primary key) partition by hash(pk) partitions 2; +insert into t1 values (1); +select create_time IS NOT NULL , update_time IS NOT NULL +from information_schema.tables +where table_schema=database() and table_name='t1'; +create_time IS NOT NULL 1 +update_time IS NOT NULL 1 +drop table t1; diff --git a/mysql-test/suite/rocksdb/t/show_table_status.test b/mysql-test/suite/rocksdb/t/show_table_status.test index 59effcc788c..19f46ddb251 100644 --- a/mysql-test/suite/rocksdb/t/show_table_status.test +++ b/mysql-test/suite/rocksdb/t/show_table_status.test @@ -1,5 +1,5 @@ --source include/have_rocksdb.inc - +--source include/have_partition.inc # # SHOW TABLE STATUS statement # @@ -62,9 +62,15 @@ CREATE TABLE `t1_new..............................................end`(a int) en INSERT INTO `t1_new..............................................end` VALUES (1); --query_vertical SELECT TABLE_SCHEMA, TABLE_NAME FROM information_schema.table_statistics WHERE TABLE_NAME = 't1_new..............................................end' DROP DATABASE `db_new..............................................end`; + --echo # --echo # MDEV-17171: Bug: RocksDB Tables do not have "Creation Date" --echo # + +--let $_mysqld_option=--rocksdb_table_dictionary_format=2 +--source include/restart_mysqld_with_option.inc + +select @@rocksdb_table_dictionary_format; use test; create table t1 (a int) engine=rocksdb; @@ -130,7 +136,8 @@ select create_time from information_schema.tables where table_schema=database() and table_name='t1'; ---source include/restart_mysqld.inc +# We stiil have $_mysqld_option=--rocksdb_table_dictionary_format=2 +--source include/restart_mysqld_with_option.inc select create_time=(select create_time from t2) /* should change */, @@ -139,3 +146,17 @@ from information_schema.tables where table_schema=database() and table_name='t1'; drop table t1, t2; + +--echo # +--echo # Check how it works for partitioned tables +--echo # +create table t1 (pk int primary key) partition by hash(pk) partitions 2; +insert into t1 values (1); + +select create_time IS NOT NULL , update_time IS NOT NULL +from information_schema.tables +where table_schema=database() and table_name='t1'; +drop table t1; + +--let $_mysqld_option= +--source include/restart_mysqld.inc diff --git a/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_table_dictionary_format_basic.result b/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_table_dictionary_format_basic.result index 825d7a7113a..c64a31f6e96 100644 --- a/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_table_dictionary_format_basic.result +++ b/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_table_dictionary_format_basic.result @@ -6,7 +6,7 @@ INSERT INTO invalid_values VALUES('\'aaa\''); SET @start_global_value = @@global.ROCKSDB_TABLE_DICTIONARY_FORMAT; SELECT @start_global_value; @start_global_value -2 +1 "Trying to set variable @@global.ROCKSDB_TABLE_DICTIONARY_FORMAT to 444. It should fail because it is readonly." SET @@global.ROCKSDB_TABLE_DICTIONARY_FORMAT = 444; ERROR HY000: Variable 'rocksdb_table_dictionary_format' is a read only variable diff --git a/storage/rocksdb/rdb_datadic.cc b/storage/rocksdb/rdb_datadic.cc index 234f7a789ff..ceb23bbdd06 100644 --- a/storage/rocksdb/rdb_datadic.cc +++ b/storage/rocksdb/rdb_datadic.cc @@ -3517,17 +3517,23 @@ bool Rdb_tbl_def::put_dict(Rdb_dict_manager *const dict, Rdb_key_def::TABLE_CREATE_TIMESTAMP_SIZE + m_key_count * Rdb_key_def::PACKED_SIZE * 2); - if (rocksdb_table_dictionary_format < - ROCKSDB_DATADIC_FORMAT_CREATE_TIMESTAMP) { - rdb_netstr_append_uint16(&indexes, Rdb_key_def::DDL_ENTRY_INDEX_VERSION_1); - // We are using old data format, which means we cannot save Create_time - // Set it to be shown as unknown right away, so that the behavior before - // server restart and after is the same. - set_create_time(0); - } - else { - rdb_netstr_append_uint16(&indexes, Rdb_key_def::DDL_ENTRY_INDEX_VERSION_2); - rdb_netstr_append_uint64(&indexes, create_time); + switch (rocksdb_table_dictionary_format) { + case ROCKSDB_DATADIC_FORMAT_INITIAL: + rdb_netstr_append_uint16(&indexes, Rdb_key_def::DDL_ENTRY_INDEX_VERSION_1); + // We are using old data format, which means we cannot save Create_time + // Set it to be shown as unknown right away, so that the behavior before + // server restart and after is the same. + set_create_time(0); + break; + case ROCKSDB_DATADIC_FORMAT_CREATE_TIMESTAMP: + rdb_netstr_append_uint16(&indexes, Rdb_key_def::DDL_ENTRY_INDEX_VERSION_2); + rdb_netstr_append_uint64(&indexes, create_time); + break; + default: + DBUG_ASSERT(0); + rdb_netstr_append_uint16(&indexes, Rdb_key_def::DDL_ENTRY_INDEX_VERSION_1); + set_create_time(0); + break; } for (uint i = 0; i < m_key_count; i++) { @@ -4068,6 +4074,7 @@ bool Rdb_ddl_manager::init(Rdb_dict_manager *const dict_arg, // NO_LINT_DEBUG sql_print_error("RocksDB: Table_store: invalid keylist for table %s", tdef->full_tablename().c_str()); + delete tdef; return true; } tdef->m_key_count = real_val_size / (Rdb_key_def::PACKED_SIZE * 2); @@ -4087,6 +4094,7 @@ bool Rdb_ddl_manager::init(Rdb_dict_manager *const dict_arg, "for Index Number (%u,%u), table %s", gl_index_id.cf_id, gl_index_id.index_id, tdef->full_tablename().c_str()); + delete tdef; return true; } if (max_index_id_in_dict < gl_index_id.index_id) { @@ -4096,6 +4104,7 @@ bool Rdb_ddl_manager::init(Rdb_dict_manager *const dict_arg, "but also found larger index id %u from dictionary. " "This should never happen and possibly a bug.", max_index_id_in_dict, gl_index_id.index_id); + delete tdef; return true; } if (!m_dict->get_cf_flags(gl_index_id.cf_id, &flags)) { @@ -4104,6 +4113,7 @@ bool Rdb_ddl_manager::init(Rdb_dict_manager *const dict_arg, "RocksDB: Could not get Column Family Flags " "for CF Number %d, table %s", gl_index_id.cf_id, tdef->full_tablename().c_str()); + delete tdef; return true; } diff --git a/storage/rocksdb/rdb_datadic.h b/storage/rocksdb/rdb_datadic.h index 9c2a96e77b7..4e10e772ecf 100644 --- a/storage/rocksdb/rdb_datadic.h +++ b/storage/rocksdb/rdb_datadic.h @@ -216,7 +216,7 @@ const uint ROCKSDB_DATADIC_FORMAT_CREATE_TIMESTAMP = 2; // Maximum possible value: const uint ROCKSDB_DATADIC_FORMAT_MAX = 2; -const uint ROCKSDB_DATADIC_FORMAT_DEFAULT = ROCKSDB_DATADIC_FORMAT_MAX; +const uint ROCKSDB_DATADIC_FORMAT_DEFAULT = ROCKSDB_DATADIC_FORMAT_INITIAL; /* @@ -523,9 +523,11 @@ class Rdb_key_def { // Data dictionary schema version. Introduce newer versions // if changing schema layout enum { + // Used when rocksdb_table_dictionary_format=ROCKSDB_DATADIC_FORMAT_INITIAL DDL_ENTRY_INDEX_VERSION_1 = 1, - // this includes a 64-bit table_creation_time at the end. - // Allowed since ROCKSDB_DATADIC_FORMAT_CREATE_TIMESTAMP. + + // Used when rocksdb_table_dictionary_format= + // ROCKSDB_DATADIC_FORMAT_CREATE_TIMESTAMP DDL_ENTRY_INDEX_VERSION_2 = 2, CF_DEFINITION_VERSION = 1, BINLOG_INFO_INDEX_NUMBER_VERSION = 1, @@ -1370,9 +1372,11 @@ class Rdb_binlog_manager { 1. Table Name => internal index id mappings key: Rdb_key_def::DDL_ENTRY_INDEX_START_NUMBER(0x1) + dbname.tablename - value: DDL_ENTRY_INDEX_VERSION_1, {cf_id, index_id}*n_indexes_of_the_table - or value: DDL_ENTRY_INDEX_VERSION_2, create_timestamp, {cf_id, index_id}* - n_indexes_of_the_table + value has two versions: + version=DDL_ENTRY_INDEX_VERSION_1, {cf_id, index_id}*n_indexes_of_the_table. + version=DDL_ENTRY_INDEX_VERSION_2, create_timestamp, {cf_id, index_id}* + n_indexes_of_the_table. + version is 2 bytes. cf_id and index_id are 4 bytes. create_timestamp is 8 bytes.

1 0

[Commits] caaf170ba42: Issue #790, MyRocks/MRR: address review input part 3
by psergey 15 Oct '19

15 Oct '19

revision-id: caaf170ba4210bf614140d88c0fa108dff063b0d (fb-prod201903-171-gcaaf170ba42) parent(s): 71f5fa77faeae64d653012db88f0a9e224227d5f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-15 20:50:58 +0300 message: Issue #790, MyRocks/MRR: address review input part 3 - in ha_rocksdb::index_end(), call mrr_free() only if we are in an MRR scan. - added a comment. - Move the "if (table->in_use->killed) return HA_ERR_QUERY_INTERRUPTED" logic --- storage/rocksdb/ha_rocksdb.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 2735d9d6adb..1ae5c1d6989 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -8013,6 +8013,9 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf, #ifndef DBUG_OFF m_keyread_only = save_keyread_only; #endif + // Due to MRR, now an index-only scan have pushed index condition. + // (If it does, we follow non-index only code path here, except that + // we don't fetch the row). bool have_icp = (pushed_idx_cond && pushed_idx_cond_keyno == active_index); if (covered_lookup && m_lock_rows == RDB_LOCK_NONE && !have_icp) { @@ -10569,7 +10572,8 @@ int ha_rocksdb::index_end() { active_index = MAX_KEY; in_range_check_pushed_down = FALSE; - mrr_free(); + if (mrr_rowid_reader) + mrr_free(); DBUG_RETURN(HA_EXIT_SUCCESS); } @@ -15520,6 +15524,8 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { while (1) { while (1) { + if (table->in_use->killed) return HA_ERR_QUERY_INTERRUPTED; + if (mrr_read_index >= mrr_n_elements) { if (mrr_rowid_reader->eof() || !mrr_n_elements) { table->status = STATUS_NOT_FOUND; // not sure if this is necessary? @@ -15527,8 +15533,6 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { return HA_ERR_END_OF_FILE; } - if (table->in_use->killed) return HA_ERR_QUERY_INTERRUPTED; - if ((rc = mrr_fill_buffer())) { if (rc == HA_ERR_END_OF_FILE) table->status = STATUS_NOT_FOUND; return rc;

1 0

[Commits] a1c29cecdb1: Issue #790, MyRocks/MRR: address review input part 3
by psergey 15 Oct '19

15 Oct '19

revision-id: a1c29cecdb17578ef6364ec9021fd9222097923c (fb-prod201903-171-ga1c29cecdb1) parent(s): 71f5fa77faeae64d653012db88f0a9e224227d5f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-10-15 20:48:46 +0300 message: Issue #790, MyRocks/MRR: address review input part 3 - in ha_rocksdb::index_end(), call mrr_free() only if we are in an MRR scan. - added a comment. --- storage/rocksdb/ha_rocksdb.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 2735d9d6adb..1ae5c1d6989 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -8013,6 +8013,9 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf, #ifndef DBUG_OFF m_keyread_only = save_keyread_only; #endif + // Due to MRR, now an index-only scan have pushed index condition. + // (If it does, we follow non-index only code path here, except that + // we don't fetch the row). bool have_icp = (pushed_idx_cond && pushed_idx_cond_keyno == active_index); if (covered_lookup && m_lock_rows == RDB_LOCK_NONE && !have_icp) { @@ -10569,7 +10572,8 @@ int ha_rocksdb::index_end() { active_index = MAX_KEY; in_range_check_pushed_down = FALSE; - mrr_free(); + if (mrr_rowid_reader) + mrr_free(); DBUG_RETURN(HA_EXIT_SUCCESS); } @@ -15520,6 +15524,8 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { while (1) { while (1) { + if (table->in_use->killed) return HA_ERR_QUERY_INTERRUPTED; + if (mrr_read_index >= mrr_n_elements) { if (mrr_rowid_reader->eof() || !mrr_n_elements) { table->status = STATUS_NOT_FOUND; // not sure if this is necessary? @@ -15527,8 +15533,6 @@ int ha_rocksdb::multi_range_read_next(char **range_info) { return HA_ERR_END_OF_FILE; } - if (table->in_use->killed) return HA_ERR_QUERY_INTERRUPTED; - if ((rc = mrr_fill_buffer())) { if (rc == HA_ERR_END_OF_FILE) table->status = STATUS_NOT_FOUND; return rc;

1 0

[Commits] MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key
by Sachin Setiya 14 Oct '19

14 Oct '19

commit 02362a77d0d458f8bbdca50f660e7bfca1470a35 Author: Sachin <sachin.setiya(a)mariadb.com> Date: Mon Oct 14 15:29:14 2019 +0530 MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key Problem:- Insert delayed is not working with Long Unique Index. It is failing with 1. INSERT DELAYED INTO t1 VALUES(); 2. INSERT DELAYED INTO t1 VALUES(1); 3. Potential Race condition When Insert DELAYED gets dup key error(After fix), And it will change original table key_info by calling re_setup_keyinfo_hash, And second thread is in check_duplicate_long_entries 4. Insert delayed into INVISIBLE COLUMN will also not work. There are 4 main issue 1. while calling make_new_field we forgot to & LONG_UNIQUE_HASH_FIELD flag into new field flags. 2. New field created created into get_local_table by make_new_field does not respect old field visibility, Assigning old field visibility will solve Problem 4 and part of problem 2. 3. As we know Problem 3 race condition is caused because table and delayed table share same key_info, So we will make a copy of original table key_info in get_local_table. 4. In parse_vcol_defs we have this code block keypart->field->vcol_info= table->field[keypart->field->field_index]->vcol_info; Which is wrong because we should not change original table->field->vcol_info with vcol_info which is create on delayed thread. diff --git a/mysql-test/main/long_unique_bugs.result b/mysql-test/main/long_unique_bugs.result index c0ba4d0b87d..16e825f0905 100644 --- a/mysql-test/main/long_unique_bugs.result +++ b/mysql-test/main/long_unique_bugs.result @@ -270,3 +270,32 @@ ERROR 42000: Specified key was too long; max key length is 1000 bytes create table t1(a int, unique(a) using hash); #BULK insert > 100 rows (MI_MIN_ROWS_TO_DISABLE_INDEXES) drop table t1; +CREATE TABLE t1 (a blob, UNIQUE(a)) ENGINE=MyISAM; +INSERT DELAYED t1 () VALUES (1); +INSERT t1 () VALUES (2); +DROP TABLE t1; +CREATE TABLE t1 (a char(50), UNIQUE(a(10)) USING HASH); +INSERT DELAYED t1 () VALUES (1); +INSERT t1 () VALUES (2); +DROP TABLE t1; +CREATE TABLE t1 ( +a CHAR(128), +b CHAR(128) AS (a), +c varchar(5000), +UNIQUE(c,b(64)) +) ENGINE=myisam; +INSERT DELAYED t1 (a,c) VALUES (1,1); +INSERT t1 (a,c) VALUES (2,2); +INSERT t1 (a,c) VALUES (3,3); +drop table t1; +create table t1(a int , b int invisible); +insert into t1 values(1); +insert delayed into t1(a,b) values(2,2); +#Should not fails +insert delayed into t1 values(2); +select a,b from t1 order by a; +a b +1 NULL +2 2 +2 NULL +DROP TABLE t1; diff --git a/mysql-test/main/long_unique_bugs.test b/mysql-test/main/long_unique_bugs.test index 13a4e1367a0..365393aa2ea 100644 --- a/mysql-test/main/long_unique_bugs.test +++ b/mysql-test/main/long_unique_bugs.test @@ -340,3 +340,35 @@ while ($count) --eval $insert_stmt --enable_query_log drop table t1; +# +# MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key +# +CREATE TABLE t1 (a blob, UNIQUE(a)) ENGINE=MyISAM; +INSERT DELAYED t1 () VALUES (1); +INSERT t1 () VALUES (2); +# Cleanup +DROP TABLE t1; +CREATE TABLE t1 (a char(50), UNIQUE(a(10)) USING HASH); +INSERT DELAYED t1 () VALUES (1); +INSERT t1 () VALUES (2); +# Cleanup +DROP TABLE t1; +CREATE TABLE t1 ( + a CHAR(128), + b CHAR(128) AS (a), + c varchar(5000), + UNIQUE(c,b(64)) +) ENGINE=myisam; +INSERT DELAYED t1 (a,c) VALUES (1,1); +--sleep 1 +INSERT t1 (a,c) VALUES (2,2); +INSERT t1 (a,c) VALUES (3,3); +drop table t1; +create table t1(a int , b int invisible); +insert into t1 values(1); +insert delayed into t1(a,b) values(2,2); +--echo #Should not fails +insert delayed into t1 values(2); +select a,b from t1 order by a; +# Cleanup +DROP TABLE t1; diff --git a/sql/field.cc b/sql/field.cc index 0eb53f40a54..d1187237f23 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -2373,8 +2373,13 @@ Field *Field::make_new_field(MEM_ROOT *root, TABLE *new_table, tmp->flags&= (NOT_NULL_FLAG | BLOB_FLAG | UNSIGNED_FLAG | ZEROFILL_FLAG | BINARY_FLAG | ENUM_FLAG | SET_FLAG | VERS_SYS_START_FLAG | VERS_SYS_END_FLAG | - VERS_UPDATE_UNVERSIONED_FLAG); + VERS_UPDATE_UNVERSIONED_FLAG | LONG_UNIQUE_HASH_FIELD); tmp->reset_fields(); + /* + Calling make_new_field will return a VISIBLE field, If caller function + wants original visibility he should change it later. + This is done because view created on invisible fields are visible. + */ tmp->invisible= VISIBLE; return tmp; } diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index fe6c5fa8ec4..d80044ec37d 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2607,6 +2607,11 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) { if (!(*field= (*org_field)->make_new_field(client_thd->mem_root, copy, 1))) goto error; + /* + We want same visibility as of original table because we are just creating + a clone for delayed insert. + */ + (*field)->invisible= (*org_field)->invisible; (*field)->unireg_check= (*org_field)->unireg_check; (*field)->orig_table= copy; // Remove connection (*field)->move_field_offset(adjust_ptrs); // Point at copy->record[0] @@ -2621,7 +2626,33 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) if (share->virtual_fields || share->default_expressions || share->default_fields) { + /* + If we have long unique table then delayed insert can modify key structure + (re/setup_keyinfo_hash_all) of original table when it gets insert error, + parse_vcol_defs will also modify key_info structure. So it is better to + clone the table->key_info for copy table. + We will not be cloning key_part_info or even changing any field ptr. + Because re/setup_keyinfo_hash_all only modify key_info array. So it will + be like having new key_info array for copy table with old key_part_info + ptr. + */ + if (share->long_unique_table) + { + KEY *key_info; + if (!(key_info= (KEY*) client_thd->alloc(share->keys*sizeof(KEY)))) + goto error; + copy->key_info= key_info; + memcpy(key_info, table->key_info, sizeof(KEY)*share->keys); + } + /* + parse_vcol_defs expects key_infos to be in user defined format. + */ + copy->setup_keyinfo_hash_all(); bool error_reported= FALSE; + /* + We won't be calling re_setup_keyinfo_hash because parse_vcol_defs changes + key_infos to storage engine format + */ if (unlikely(parse_vcol_defs(client_thd, client_thd->mem_root, copy, &error_reported, VCOL_INIT_DEPENDENCY_FAILURE_IS_WARNING))) diff --git a/sql/table.cc b/sql/table.cc index 718d0dce072..d0fcd449098 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1223,7 +1223,16 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table, new (mem_root) Item_field(thd, keypart->field), new (mem_root) Item_int(thd, length)); list_item->fix_fields(thd, NULL); - keypart->field->vcol_info= + /* + Do not change the vcol_info when vcol_info->expr is not NULL + This will happen in the case of Delayed_insert::get_local_table() + And if we change the vcol_info in Delayed insert , then original + table field->vcol_info will be created on delayed insert thread + mem_root. + */ + if (!keypart->field->vcol_info || + !keypart->field->vcol_info->expr) + keypart->field->vcol_info= table->field[keypart->field->field_index]->vcol_info; } else @@ -9084,6 +9093,26 @@ void re_setup_keyinfo_hash(KEY *key_info) key_info->ext_key_parts= 1; key_info->flags&= ~HA_NOSAME; } + +/* + call setup_keyinfo_hash for all keys in table + */ +void TABLE::setup_keyinfo_hash_all() +{ + for (uint i= 0; i < s->keys; i++) + if (key_info[i].algorithm == HA_KEY_ALG_LONG_HASH) + setup_keyinfo_hash(&key_info[i]); +} + +/* + call re_setup_keyinfo_hash for all keys in table + */ +void TABLE::re_setup_keyinfo_hash_all() +{ + for (uint i= 0; i < s->keys; i++) + if (key_info[i].algorithm == HA_KEY_ALG_LONG_HASH) + re_setup_keyinfo_hash(&key_info[i]); +} /** @brief clone of current handler. Creates a clone of handler used in update for diff --git a/sql/table.h b/sql/table.h index 1c721624d5d..76d11bbd604 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1612,6 +1612,8 @@ struct TABLE void vers_update_fields(); void vers_update_end(); void find_constraint_correlated_indexes(); + void setup_keyinfo_hash_all(); + void re_setup_keyinfo_hash_all(); void clone_handler_for_update(); void delete_update_handler(); -- Regards Sachin Setiya Software Engineer at MariaDB

1 0